From aa06ad4a5f6851a5ffab3de2d2d2dea4bc864f45 Mon Sep 17 00:00:00 2001 From: eileen Date: Fri, 25 Sep 2020 11:01:09 +1200 Subject: [PATCH] [REF] Fix Event location to create it's locations directly rather than via shared methods This form does extra work in order to call CRM_Core_BAO_Block::create and CRM_Core_BAO_Address::create - but they have extra handling that only exists for this code place. So, we are making 2 functions more complex in order to inter-relate them. This simplifies the CRM_Event_Form_ManageEvent_Location and I will do follow on cleanup once merged. Note I previously added tests to CRM_Event_Form_ManageEvent_LocationTest specifically to cover this (in a recent previous refactor --- CRM/Core/BAO/Location.php | 1 + CRM/Core/BAO/Phone.php | 1 + CRM/Event/Form/ManageEvent/Location.php | 35 ++-- Civi/Api4/LocBlock.php | 22 +++ tests/phpunit/CRM/Core/BAO/LocationTest.php | 159 ------------------ .../Event/Form/ManageEvent/LocationTest.php | 108 ++++++++++-- tests/phpunit/api/v3/ACLPermissionTest.php | 1 + 7 files changed, 142 insertions(+), 185 deletions(-) create mode 100644 Civi/Api4/LocBlock.php diff --git a/CRM/Core/BAO/Location.php b/CRM/Core/BAO/Location.php index add13da479..911862b8fc 100644 --- a/CRM/Core/BAO/Location.php +++ b/CRM/Core/BAO/Location.php @@ -73,6 +73,7 @@ class CRM_Core_BAO_Location extends CRM_Core_DAO { * @return int */ public static function createLocBlock($location, $entityElements) { + CRM_Core_Error::deprecatedFunctionWarning('Use LocBlock api'); $locId = self::findExisting($entityElements); $locBlock = []; diff --git a/CRM/Core/BAO/Phone.php b/CRM/Core/BAO/Phone.php index dac8c9fa5f..a9303ed0cb 100644 --- a/CRM/Core/BAO/Phone.php +++ b/CRM/Core/BAO/Phone.php @@ -28,6 +28,7 @@ class CRM_Core_BAO_Phone extends CRM_Core_DAO_Phone { * * @return object * @throws API_Exception + * @throws \CRM_Core_Exception */ public static function create($params) { // Ensure mysql phone function exists diff --git a/CRM/Event/Form/ManageEvent/Location.php b/CRM/Event/Form/ManageEvent/Location.php index a1a79396a0..aa5aca492a 100644 --- a/CRM/Event/Form/ManageEvent/Location.php +++ b/CRM/Event/Form/ManageEvent/Location.php @@ -10,6 +10,10 @@ */ use Civi\Api4\Event; +use Civi\Api4\LocBlock; +use Civi\Api4\Email; +use Civi\Api4\Phone; +use Civi\Api4\Address; /** * @@ -227,14 +231,10 @@ class CRM_Event_Form_ManageEvent_Location extends CRM_Event_Form_ManageEvent { CRM_Event_BAO_Event::deleteEventLocBlock($this->_oldLocBlockId, $this->_id); } - // get ready with location block params - $params['entity_table'] = 'civicrm_event'; - $params['entity_id'] = $this->_id; - $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'], @@ -249,18 +249,21 @@ 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'); - } } - - $params['loc_block_id'] = CRM_Core_BAO_Location::createLocBlock($location, [ - 'entity_table' => $params['entity_table'], - 'entity_id' => $params['entity_id'], - ]); + $addresses = Address::save(FALSE)->setRecords($params['address'])->execute(); + $emails = Email::save(FALSE)->setRecords($params['email'])->execute(); + $phones = Phone::save(FALSE)->setRecords($params['phone'])->execute(); + + $params['loc_block_id'] = LocBlock::save(FALSE)->setRecords([ + [ + 'email_id' => $emails[0]['id'] ?? NULL, + 'address_id' => $addresses[0]['id'] ?? NULL, + 'phone_id' => $phones[0]['id'] ?? NULL, + 'email_2_id' => $emails[1]['id'] ?? NULL, + 'address_2_id' => $addresses[1]['id'] ?? NULL, + 'phone_2_id' => $phones[1]['id'] ?? NULL, + ], + ])->execute()->first()['id']; // finally update event params $params['id'] = $this->_id; diff --git a/Civi/Api4/LocBlock.php b/Civi/Api4/LocBlock.php new file mode 100644 index 0000000000..6b0ac77de9 --- /dev/null +++ b/Civi/Api4/LocBlock.php @@ -0,0 +1,22 @@ +contactDelete($contactId); } - /** - * Create() method - * create various elements of location block - * with civicrm_loc_block - */ - public function testCreateWithLocBlock() { - $this->_contactId = $this->individualCreate(); - $event = $this->eventCreate(); - $params = [ - 'address' => [ - '1' => [ - 'street_address' => 'Saint Helier St', - 'supplemental_address_1' => 'Hallmark Ct', - 'supplemental_address_2' => 'Jersey Village', - 'supplemental_address_3' => 'My Town', - 'city' => 'Newark', - 'postal_code' => '01903', - 'country_id' => 1228, - 'state_province_id' => 1029, - 'geo_code_1' => '18.219023', - 'geo_code_2' => '-105.00973', - 'is_primary' => 1, - 'location_type_id' => 1, - ], - ], - 'email' => [ - '1' => [ - 'email' => 'john.smith@example.org', - 'is_primary' => 1, - 'location_type_id' => 1, - ], - ], - 'phone' => [ - '1' => [ - 'phone_type_id' => 1, - 'phone' => '303443689', - 'is_primary' => 1, - 'location_type_id' => 1, - ], - '2' => [ - 'phone_type_id' => 2, - 'phone' => '9833910234', - 'location_type_id' => 1, - ], - ], - 'im' => [ - '1' => [ - 'name' => 'jane.doe', - 'provider_id' => 1, - 'location_type_id' => 1, - 'is_primary' => 1, - ], - ], - ]; - - $params['entity_id'] = $event['id']; - $params['entity_table'] = 'civicrm_event'; - - //create location block. - //with various element of location block - //like address, phone, email, im. - 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' => CRM_Core_BAO_Location::createLocBlock($location, [ - 'entity_id' => $event['id'], - 'entity_table' => 'civicrm_event', - ]), - ]; - - CRM_Event_BAO_Event::add($eventParams); - - //Now check DB for location block - - $this->assertDBCompareValue('CRM_Event_DAO_Event', - $event['id'], - 'loc_block_id', - 'id', - $eventParams['loc_block_id'], - 'Checking database for the record.' - ); - $locElementIds = []; - $locParams = ['id' => $eventParams['loc_block_id']]; - CRM_Core_DAO::commonRetrieve('CRM_Core_DAO_LocBlock', - $locParams, - $locElementIds - ); - - //Now check DB for location elements. - $searchParams = [ - 'id' => $locElementIds['address_id'] ?? NULL, - 'location_type_id' => 1, - 'is_primary' => 1, - ]; - $compareParams = [ - 'street_address' => 'Saint Helier St', - 'supplemental_address_1' => 'Hallmark Ct', - 'supplemental_address_2' => 'Jersey Village', - 'supplemental_address_3' => 'My Town', - 'city' => 'Newark', - 'postal_code' => '01903', - 'country_id' => 1228, - 'state_province_id' => 1029, - 'geo_code_1' => '18.219023', - 'geo_code_2' => '-105.00973', - ]; - $this->assertDBCompareValues('CRM_Core_DAO_Address', $searchParams, $compareParams); - - $searchParams = [ - 'id' => $locElementIds['email_id'] ?? NULL, - 'location_type_id' => 1, - 'is_primary' => 1, - ]; - $compareParams = ['email' => 'john.smith@example.org']; - $this->assertDBCompareValues('CRM_Core_DAO_Email', $searchParams, $compareParams); - - $searchParams = [ - 'id' => $locElementIds['phone_id'] ?? NULL, - 'location_type_id' => 1, - 'is_primary' => 1, - 'phone_type_id' => 1, - ]; - $compareParams = ['phone' => '303443689']; - $this->assertDBCompareValues('CRM_Core_DAO_Phone', $searchParams, $compareParams); - - $searchParams = [ - 'id' => $locElementIds['phone_2_id'] ?? NULL, - 'location_type_id' => 1, - 'phone_type_id' => 2, - ]; - $compareParams = ['phone' => '9833910234']; - $this->assertDBCompareValues('CRM_Core_DAO_Phone', $searchParams, $compareParams); - - $searchParams = [ - 'id' => $locElementIds['im_id'] ?? NULL, - 'location_type_id' => 1, - 'is_primary' => 1, - ]; - $compareParams = [ - 'name' => 'jane.doe', - 'provider_id' => 1, - ]; - $this->assertDBCompareValues('CRM_Core_DAO_IM', $searchParams, $compareParams); - - // Cleanup. - CRM_Core_BAO_Location::deleteLocBlock($eventParams['loc_block_id']); - $this->eventDelete($event['id']); - $this->contactDelete($this->_contactId); - } - /** * GetValues() method * get the values of various location elements diff --git a/tests/phpunit/CRM/Event/Form/ManageEvent/LocationTest.php b/tests/phpunit/CRM/Event/Form/ManageEvent/LocationTest.php index 1767a26c81..8ff04280db 100644 --- a/tests/phpunit/CRM/Event/Form/ManageEvent/LocationTest.php +++ b/tests/phpunit/CRM/Event/Form/ManageEvent/LocationTest.php @@ -20,22 +20,96 @@ class CRM_Event_Form_ManageEvent_LocationTest extends CiviUnitTestCase { */ public function testSubmit() { $eventID = (int) $this->eventCreate()['id']; - $form = $this->getFormObject('CRM_Event_Form_ManageEvent_Location', $this->getFormValues()); - $form->set('id', $eventID); - $form->preProcess(); - $form->buildQuickForm(); - $form->postProcess(); + $this->submitForm([], $eventID); $this->assertCorrectEmails($eventID); // Now do it again to see if it gets messed with. - $form = $this->getFormObject('CRM_Event_Form_ManageEvent_Location', array_merge($this->getFormValues(), ['loc_event_id' => $this->ids['LocBlock'][0]])); - $form->set('id', $eventID); - $form->preProcess(); - $form->buildQuickForm(); - $form->postProcess(); + $this->submitForm(['loc_event_id' => $this->ids['LocBlock'][0]], $eventID); $this->assertCorrectEmails($eventID); } + /** + * Create() method + * create various elements of location block + * with civicrm_loc_block + */ + public function testCreateWithLocBlock() { + $eventID = (int) $this->eventCreate()['id']; + $this->submitForm([ + 'address' => [ + '1' => [ + 'street_address' => 'Saint Helier St', + 'supplemental_address_1' => 'Hallmark Ct', + 'supplemental_address_2' => 'Jersey Village', + 'supplemental_address_3' => 'My Town', + 'city' => 'Newark', + 'postal_code' => '01903', + 'country_id' => 1228, + 'state_province_id' => 1029, + 'geo_code_1' => '18.219023', + 'geo_code_2' => '-105.00973', + 'is_primary' => 1, + 'location_type_id' => 1, + ], + ], + 'email' => [ + '1' => [ + 'email' => 'john.smith@example.org', + 'is_primary' => 1, + 'location_type_id' => 1, + ], + ], + 'phone' => [ + '1' => [ + 'phone_type_id' => 1, + 'phone' => '303443689', + 'is_primary' => 1, + 'location_type_id' => 1, + ], + '2' => [ + 'phone_type_id' => 2, + 'phone' => '9833910234', + 'location_type_id' => 1, + ], + ], + ], $eventID); + + //Now check DB for location block + + $locationBlock = Event::get() + ->addWhere('id', '=', $eventID) + ->setSelect(['loc_block.*', 'loc_block_id']) + ->execute()->first(); + + $address = $this->callAPISuccessGetSingle('Address', ['id' => $locationBlock['loc_block.address_id']]); + + $this->assertEquals([ + 'id' => $address['id'], + 'location_type_id' => '1', + 'is_primary' => '1', + 'is_billing' => '0', + 'street_address' => 'Saint Helier St', + 'supplemental_address_1' => 'Hallmark Ct', + 'supplemental_address_2' => 'Jersey Village', + 'supplemental_address_3' => 'My Town', + 'city' => 'Newark', + 'postal_code' => '01903', + 'country_id' => 1228, + 'state_province_id' => 1029, + 'geo_code_1' => '18.219023', + 'geo_code_2' => '-105.00973', + 'manual_geo_code' => '0', + ], $address); + + $this->callAPISuccessGetSingle('Email', ['id' => $locationBlock['loc_block.email_id'], 'email' => 'john.smith@example.org']); + $this->callAPISuccessGetSingle('Phone', ['id' => $locationBlock['loc_block.phone_id'], 'phone' => '303443689']); + $this->callAPISuccessGetSingle('Phone', ['id' => $locationBlock['loc_block.phone_2_id'], 'phone' => '9833910234']); + + // Cleanup. + CRM_Core_BAO_Location::deleteLocBlock($locationBlock['loc_block_id']); + $this->eventDelete($eventID); + } + /** * Get the values to submit for the form. * @@ -120,4 +194,18 @@ class CRM_Event_Form_ManageEvent_LocationTest extends CiviUnitTestCase { return $emails; } + /** + * @param array $formValues + * @param int $eventID + * + * @throws \CRM_Core_Exception + */ + protected function submitForm(array $formValues, int $eventID): void { + $form = $this->getFormObject('CRM_Event_Form_ManageEvent_Location', array_merge($this->getFormValues(), $formValues)); + $form->set('id', $eventID); + $form->preProcess(); + $form->buildQuickForm(); + $form->postProcess(); + } + } diff --git a/tests/phpunit/api/v3/ACLPermissionTest.php b/tests/phpunit/api/v3/ACLPermissionTest.php index fdbdabea1c..558e99a484 100644 --- a/tests/phpunit/api/v3/ACLPermissionTest.php +++ b/tests/phpunit/api/v3/ACLPermissionTest.php @@ -43,6 +43,7 @@ class api_v3_ACLPermissionTest extends CiviUnitTestCase { $this->cleanUpAfterACLs(); $tablesToTruncate = [ 'civicrm_contact', + 'civicrm_address', 'civicrm_group_contact', 'civicrm_group', 'civicrm_acl', -- 2.25.1