From 7ecf62753884d9c9ae982f458023ea76d4d408e7 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Tue, 6 Jan 2015 22:58:05 +1300 Subject: [PATCH] CRM-15786 refactor relationship create function to createMultiple & 'stput std create in it's place Conflicts: CRM/Contact/BAO/Relationship.php tests/phpunit/api/v3/AddressTest.php --- CRM/Contact/BAO/Contact/Utils.php | 2 +- CRM/Contact/BAO/Relationship.php | 171 ++++++++++++------ CRM/Contact/Form/Relationship.php | 2 +- CRM/Contact/Import/Parser/Contact.php | 2 +- CRM/Contact/Page/AJAX.php | 2 +- CRM/Contribute/Form/Contribution/Confirm.php | 2 +- CRM/Core/BAO/Address.php | 2 +- api/v3/Relationship.php | 19 +- .../BAO/ContactType/RelationshipTest.php | 12 +- tests/phpunit/api/v3/AddressTest.php | 8 +- 10 files changed, 140 insertions(+), 82 deletions(-) diff --git a/CRM/Contact/BAO/Contact/Utils.php b/CRM/Contact/BAO/Contact/Utils.php index 3a463e759e..e0964e61ac 100644 --- a/CRM/Contact/BAO/Contact/Utils.php +++ b/CRM/Contact/BAO/Contact/Utils.php @@ -321,7 +321,7 @@ UNION ); list($valid, $invalid, $duplicate, $saved, $relationshipIds - ) = CRM_Contact_BAO_Relationship::create($relationshipParams, $cid); + ) = CRM_Contact_BAO_Relationship::createMultiple($relationshipParams, $cid); // In case we change employer, clean previous employer related records. diff --git a/CRM/Contact/BAO/Relationship.php b/CRM/Contact/BAO/Relationship.php index e8faf2c6d1..ec2407a678 100644 --- a/CRM/Contact/BAO/Relationship.php +++ b/CRM/Contact/BAO/Relationship.php @@ -41,9 +41,33 @@ class CRM_Contact_BAO_Relationship extends CRM_Contact_DAO_Relationship { */ const PAST = 1, DISABLED = 2, CURRENT = 4, INACTIVE = 8; + /** + * Create function. (Use the API instead) + * Note that the previous create function has been renamed 'createMultiple' + * and this is new in 4.6 + * All existing calls have been changed to createMultiple except the api call - however, it is recommended + * that you call that as the end to end testing here is based on the api & refactoring may still be done + * @param $params + * @return \CRM_Contact_BAO_Relationship + * @throws \CRM_Core_Exception + */ + public static function create(&$params) { + self::setContactABFromIDs($params); + if (self::checkDuplicateRelationship($params, $params['contact_id_a'], $params['contact_id_b'], CRM_Utils_Array::value('id', $params, 0))) { + throw new CRM_Core_Exception('Duplicate Relationship'); + } + if (self::checkValidRelationship($params, $params, 0)) { + throw new CRM_Core_Exception('Invalide Relationship'); + } + $relationship = self::add($params); + self::addRecent($params, $relationship); + return $relationship; + } /** * Takes an associative array and creates a relationship object - * + * @deprecated For single creates use the api instead (it's tested). + * For multiple a new variant of this function needs to be written and migrated to as this is a bit + * nasty * * @param array $params (reference ) an assoc array of name/value pairs * @param array $ids the array that holds all the db ids @@ -53,7 +77,7 @@ class CRM_Contact_BAO_Relationship extends CRM_Contact_DAO_Relationship { * @return CRM_Contact_BAO_Relationship object * @static */ - public static function create(&$params, $ids = array()) { + public static function createMultiple(&$params, $ids = array()) { $valid = $invalid = $duplicate = $saved = 0; $relationships = $relationshipIds = array(); $relationshipId = CRM_Utils_Array::value('relationship', $ids, CRM_Utils_Array::value('id', $params)); @@ -102,8 +126,8 @@ class CRM_Contact_BAO_Relationship extends CRM_Contact_DAO_Relationship { $duplicate++; continue; } - - $relationship = self::add($params, $ids, $key); + self::setContactABFromIDs($params, $ids, $key); + $relationship = self::add($params); $relationshipIds[] = $relationship->id; $relationships[$relationship->id] = $relationship; $valid++; @@ -148,39 +172,7 @@ class CRM_Contact_BAO_Relationship extends CRM_Contact_DAO_Relationship { // do not add to recent items for import, CRM-4399 if (!(!empty($params['skipRecentView']) || $invalid || $duplicate)) { - $url = CRM_Utils_System::url('civicrm/contact/view/rel', - "action=view&reset=1&id={$relationship->id}&cid={$relationship->contact_id_a}&context=home" - ); - - - $session = CRM_Core_Session::singleton(); - $recentOther = array(); - if (($session->get('userID') == $relationship->contact_id_a) || - CRM_Contact_BAO_Contact_Permission::allow($relationship->contact_id_a, CRM_Core_Permission::EDIT) - ) { - $rType = substr(CRM_Utils_Array::value('relationship_type_id', $params), -3); - $recentOther = array( - 'editUrl' => CRM_Utils_System::url('civicrm/contact/view/rel', - "action=update&reset=1&id={$relationship->id}&cid={$relationship->contact_id_a}&rtype={$rType}&context=home" - ), - 'deleteUrl' => CRM_Utils_System::url('civicrm/contact/view/rel', - "action=delete&reset=1&id={$relationship->id}&cid={$relationship->contact_id_a}&rtype={$rType}&context=home" - ), - ); - } - - $title = CRM_Contact_BAO_Contact::displayName($relationship->contact_id_a) . ' (' . CRM_Core_DAO::getFieldValue('CRM_Contact_DAO_RelationshipType', - $relationship->relationship_type_id, 'label_a_b' - ) . ' ' . CRM_Contact_BAO_Contact::displayName($relationship->contact_id_b) . ')'; - // add the recently created Relationship - CRM_Utils_Recent::add($title, - $url, - $relationship->id, - 'Relationship', - $relationship->contact_id_a, - NULL, - $recentOther - ); + self::addRecent($params, $relationship); } return array($valid, $invalid, $duplicate, $saved, $relationshipIds, $relationships); @@ -207,27 +199,25 @@ class CRM_Contact_BAO_Relationship extends CRM_Contact_DAO_Relationship { //@todo hook are called from create and add - remove one CRM_Utils_Hook::pre($hook , 'Relationship', $relationshipId, $params); + self::setContactABFromIDs($params); $relationshipTypes = CRM_Utils_Array::value('relationship_type_id', $params); // explode the string with _ to get the relationship type id // and to know which contact has to be inserted in // contact_id_a and which one in contact_id_b - list($type, $first, $second) = explode('_', $relationshipTypes); - - ${'contact_' . $first} = CRM_Utils_Array::value('contact', $ids); - ${'contact_' . $second} = $contactId; + list($type) = explode('_', $relationshipTypes); // check if the relationship type is Head of Household then update the // household's primary contact with this contact. if ($type == 6) { - CRM_Contact_BAO_Household::updatePrimaryContact($contact_b, $contact_a); + CRM_Contact_BAO_Household::updatePrimaryContact($params['contact_id_b'], $params['contact_id_a']); } $relationship = new CRM_Contact_BAO_Relationship(); //@todo this code needs to be updated for the possibility that not all fields are set // (update) - $relationship->contact_id_b = $contact_b; - $relationship->contact_id_a = $contact_a; + $relationship->contact_id_b = $params['contact_id_b']; + $relationship->contact_id_a = $params['contact_id_a']; $relationship->relationship_type_id = $type; $relationship->id = $relationshipId; @@ -263,6 +253,85 @@ class CRM_Contact_BAO_Relationship extends CRM_Contact_DAO_Relationship { return $relationship; } + + /** + * Add relationship to recent links + * @param array $params + * @param CRM_Contact_DAO_Relationship $relationship + */ + public static function addRecent($params, $relationship) { + $url = CRM_Utils_System::url('civicrm/contact/view/rel', + "action=view&reset=1&id={$relationship->id}&cid={$relationship->contact_id_a}&context=home" + ); + $session = CRM_Core_Session::singleton(); + $recentOther = array(); + if (($session->get('userID') == $relationship->contact_id_a) || + CRM_Contact_BAO_Contact_Permission::allow($relationship->contact_id_a, CRM_Core_Permission::EDIT) + ) { + $rType = substr(CRM_Utils_Array::value('relationship_type_id', $params), -3); + $recentOther = array( + 'editUrl' => CRM_Utils_System::url('civicrm/contact/view/rel', + "action=update&reset=1&id={$relationship->id}&cid={$relationship->contact_id_a}&rtype={$rType}&context=home" + ), + 'deleteUrl' => CRM_Utils_System::url('civicrm/contact/view/rel', + "action=delete&reset=1&id={$relationship->id}&cid={$relationship->contact_id_a}&rtype={$rType}&context=home" + ), + ); + } + $title = CRM_Contact_BAO_Contact::displayName($relationship->contact_id_a) . ' (' . CRM_Core_DAO::getFieldValue('CRM_Contact_DAO_RelationshipType', + $relationship->relationship_type_id, 'label_a_b' + ) . ' ' . CRM_Contact_BAO_Contact::displayName($relationship->contact_id_b) . ')'; + + CRM_Utils_Recent::add($title, + $url, + $relationship->id, + 'Relationship', + $relationship->contact_id_a, + NULL, + $recentOther + ); + } + + /** + * Resolve passed in contact IDs to contact_id_a & contact_id_b + * @param $params + * @param array $ids + * @param null $contactID + * @throws \CRM_Core_Exception + */ + public static function setContactABFromIDs(&$params, $ids = array(), $contactID = NULL) { + if (!empty($params['contact_id_a']) && !empty($params['contact_id_b'])) { + return; + } + if (empty($ids['contact'])) { + if (!empty($params['id'])) { + //let's load the missing ids here since other things tend to rely on them. + $fieldsToFill = array('contact_id_a', 'contact_id_b', 'relationship_type_id'); + $result = CRM_Core_DAO::executeQuery("SELECT " . implode(',', $fieldsToFill) . " FROM civicrm_relationship WHERE id = %1", array('Integer', $params['id']))->fetchRow(); + foreach ($fieldsToFill as $field) { + $params[$field] = !empty($params[$field]) ? $params[$field] : $result->$field; + } + return; + } + throw new CRM_Core_Exception('Cannot create relationship, insufficient contact IDs provided'); + } + $relationshipTypes = CRM_Utils_Array::value('relationship_type_id', $params); + list($relationshipTypeID, $first, $second) = explode('_', $relationshipTypes); + if (empty($params['relationship_type_id'])) { + $params['relationship_type_id'] = $relationshipTypeID; + } + foreach (array('a', 'b') as $contactLetter) { + if (empty($params['contact_' . $contactLetter])) { + if ($first == $contactLetter) { + $params['contact_id_' . $contactLetter] = CRM_Utils_Array::value('contact', $ids); + } + else { + $params['contact_id_' . $contactLetter] = $contactID; + } + } + } + } + /** * Specifiy defaults for creating a relationship * @@ -616,17 +685,15 @@ class CRM_Contact_BAO_Relationship extends CRM_Contact_DAO_Relationship { @access public * @static */ - public static function checkValidRelationship(&$params, &$ids, $contactId) { + public static function checkValidRelationship($params, $ids, $contactId) { $errors = ''; - + self::setContactABFromIDs($params, $ids, $contactId); // get the string of relationship type $relationshipTypes = CRM_Utils_Array::value('relationship_type_id', $params); - list($type, $first, $second) = explode('_', $relationshipTypes); - ${'contact_' . $first} = CRM_Utils_Array::value('contact', $ids); - ${'contact_' . $second} = $contactId; + list($type) = explode('_', $relationshipTypes); // function to check if the relationship selected is correct // i.e. employer relationship can exit between Individual and Organization (not between Individual and Individual) - if (!CRM_Contact_BAO_Relationship::checkRelationshipType($contact_a, $contact_b, $type)) { + if (!CRM_Contact_BAO_Relationship::checkRelationshipType($params['contact_id_a'], $params['contact_id_b'], $type)) { $errors = 'Please select valid relationship between these two contacts.'; } return $errors; @@ -657,7 +724,7 @@ WHERE relationship_type_id = " . CRM_Utils_Type::escape($type, 'Integer'); * supports date arrays BAO has increasingly standardised to ISO format * so I believe this function should support ISO rather than make API * format it - however, need to support array format for now to avoid breakage - * @ time of writing this function is called from Relationship::create (twice) + * @ time of writing this function is called from Relationship::createMultiple (twice) * CRM_BAO_Contact_Utils::clearCurrentEmployer (seemingly without dates) * CRM_Contact_Form_Task_AddToOrganization::postProcess & * CRM_Contact_Form_Task_AddToHousehold::postProcess @@ -1479,7 +1546,7 @@ cr.relationship_type_id IN (%2) AND cr.is_permission_a_b = 1 AND IF(cr.end_date IS NULL, 1, (DATEDIFF( CURDATE( ), cr.end_date ) <= 0)) AND cr.is_active = 1 AND -cc.id = cr.contact_id_b AND +cc.id = cr.contact_id_b AND cc.is_deleted = 0"; if (!empty($name)) { diff --git a/CRM/Contact/Form/Relationship.php b/CRM/Contact/Form/Relationship.php index cfef2fbca0..4a3ac7d5f1 100644 --- a/CRM/Contact/Form/Relationship.php +++ b/CRM/Contact/Form/Relationship.php @@ -458,7 +458,7 @@ class CRM_Contact_Form_Relationship extends CRM_Core_Form { ); // Save relationships - list($valid, $invalid, $duplicate, $saved, $relationshipIds) = CRM_Contact_BAO_Relationship::create($params, $ids); + list($valid, $invalid, $duplicate, $saved, $relationshipIds) = CRM_Contact_BAO_Relationship::createMultiple($params, $ids); // if this is called from case view, //create an activity for case role removal.CRM-4480 diff --git a/CRM/Contact/Import/Parser/Contact.php b/CRM/Contact/Import/Parser/Contact.php index 89a2057b8a..c5b536e606 100644 --- a/CRM/Contact/Import/Parser/Contact.php +++ b/CRM/Contact/Import/Parser/Contact.php @@ -905,7 +905,7 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Contact_Import_Parser { 'contact' => $primaryContactId, ); - list($valid, $invalid, $duplicate, $saved, $relationshipIds) = CRM_Contact_BAO_Relationship::create($relationParams, $relationIds); + list($valid, $invalid, $duplicate, $saved, $relationshipIds) = CRM_Contact_BAO_Relationship::createMultiple($relationParams, $relationIds); if ($valid || $duplicate) { $relationIds['contactTarget'] = $relContactId; diff --git a/CRM/Contact/Page/AJAX.php b/CRM/Contact/Page/AJAX.php index 3bb980db4e..74ec995e98 100644 --- a/CRM/Contact/Page/AJAX.php +++ b/CRM/Contact/Page/AJAX.php @@ -296,7 +296,7 @@ class CRM_Contact_Page_AJAX { } // create new or update existing relationship - $return = CRM_Contact_BAO_Relationship::create($relationParams, $relationIds); + $return = CRM_Contact_BAO_Relationship::createMultiple($relationParams, $relationIds); if (!empty($return[4][0])) { $relationshipID = $return[4][0]; diff --git a/CRM/Contribute/Form/Contribution/Confirm.php b/CRM/Contribute/Form/Contribution/Confirm.php index f3695f247c..f44d42ce95 100644 --- a/CRM/Contribute/Form/Contribution/Confirm.php +++ b/CRM/Contribute/Form/Contribution/Confirm.php @@ -1659,7 +1659,7 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr // create relationship $relParams['contact_check'][$orgID] = 1; $cid = array('contact' => $contactID); - CRM_Contact_BAO_Relationship::create($relParams, $cid); + CRM_Contact_BAO_Relationship::createMultiple($relParams, $cid); // if multiple match - send a duplicate alert if ($dupeIDs && (count($dupeIDs) > 1)) { diff --git a/CRM/Core/BAO/Address.php b/CRM/Core/BAO/Address.php index 2df8a1235c..589d334546 100644 --- a/CRM/Core/BAO/Address.php +++ b/CRM/Core/BAO/Address.php @@ -1028,7 +1028,7 @@ SELECT is_primary, list($valid, $invalid, $duplicate, $saved, $relationshipIds - ) = CRM_Contact_BAO_Relationship::create($relationshipParams, $cid); + ) = CRM_Contact_BAO_Relationship::createMultiple($relationshipParams, $cid); } /** diff --git a/api/v3/Relationship.php b/api/v3/Relationship.php index 2a0959cfb6..85b7e6196d 100644 --- a/api/v3/Relationship.php +++ b/api/v3/Relationship.php @@ -63,27 +63,16 @@ function civicrm_api3_relationship_create($params) { } $values['relationship_type_id'] = $values['relationship_type_id'] . '_a_b'; - if(!empty($params['contact_id_b'])){ - $values['contact_check'] = array($params['contact_id_b'] => $params['contact_id_b']); - } - if(!empty($values['contact_id_a'])){ - $ids['contact'] = $values['contact_id_a']; - } - $relationshipBAO = CRM_Contact_BAO_Relationship::create($values, $ids); - if ($relationshipBAO[1]) { - throw new API_Exception('Relationship is not valid'); - } - elseif ($relationshipBAO[2]) { - throw new API_Exception('Relationship already exists'); - } + $relationshipBAO = CRM_Contact_BAO_Relationship::create($values); + // Handle related memberships CRM-13652 if (!empty($params['contact_id_a'])) { CRM_Contact_BAO_Relationship::relatedMemberships($params['contact_id_a'], $values, $ids, $action); } - $id = $relationshipBAO[4][0]; + $id = $relationshipBAO->id; $values = array(); - _civicrm_api3_object_to_array($relationshipBAO[5][$id], $values[$id]); + _civicrm_api3_object_to_array($relationshipBAO, $values[$id]); return civicrm_api3_create_success($values, $params, 'relationship', 'create'); } diff --git a/tests/phpunit/CRM/Contact/BAO/ContactType/RelationshipTest.php b/tests/phpunit/CRM/Contact/BAO/ContactType/RelationshipTest.php index e5604efc72..f3841f8da9 100644 --- a/tests/phpunit/CRM/Contact/BAO/ContactType/RelationshipTest.php +++ b/tests/phpunit/CRM/Contact/BAO/ContactType/RelationshipTest.php @@ -171,7 +171,7 @@ DELETE FROM civicrm_contact_type ); $ids = array('contact' => $this->individual); - list($valid, $invalid, $duplicate, $saved, $relationshipIds) = CRM_Contact_BAO_Relationship::create($params, $ids); + list($valid, $invalid, $duplicate, $saved, $relationshipIds) = CRM_Contact_BAO_Relationship::createMultiple($params, $ids); $this->assertEquals($invalid, 1, 'In line ' . __LINE__); $this->assertEquals(empty($relationshipIds), TRUE, 'In line ' . __LINE__); @@ -199,7 +199,7 @@ DELETE FROM civicrm_contact_type ); $ids = array('contact' => $this->indivi_parent); - list($valid, $invalid, $duplicate, $saved, $relationshipIds) = CRM_Contact_BAO_Relationship::create($params, $ids); + list($valid, $invalid, $duplicate, $saved, $relationshipIds) = CRM_Contact_BAO_Relationship::createMultiple($params, $ids); $this->assertEquals($invalid, 1, 'In line ' . __LINE__); $this->assertEquals(empty($relationshipIds), TRUE, 'In line ' . __LINE__); @@ -224,7 +224,7 @@ DELETE FROM civicrm_contact_type ); $ids = array('contact' => $this->indivi_parent); - list($valid, $invalid, $duplicate, $saved, $relationshipIds) = CRM_Contact_BAO_Relationship::create($params, $ids); + list($valid, $invalid, $duplicate, $saved, $relationshipIds) = CRM_Contact_BAO_Relationship::createMultiple($params, $ids); $this->assertEquals($invalid, 1, 'In line ' . __LINE__); $this->assertEquals(empty($relationshipIds), TRUE, 'In line ' . __LINE__); @@ -253,7 +253,7 @@ DELETE FROM civicrm_contact_type 'contact_check' => array($this->indivi_parent => $this->indivi_parent), ); $ids = array('contact' => $this->individual); - list($valid, $invalid, $duplicate, $saved, $relationshipIds) = CRM_Contact_BAO_Relationship::create($params, $ids); + list($valid, $invalid, $duplicate, $saved, $relationshipIds) = CRM_Contact_BAO_Relationship::createMultiple($params, $ids); $this->assertEquals($valid, 1, 'In line ' . __LINE__); $this->assertEquals(empty($relationshipIds), FALSE, 'In line ' . __LINE__); @@ -282,7 +282,7 @@ DELETE FROM civicrm_contact_type 'contact_check' => array($this->indivi_student => 1), ); $ids = array('contact' => $this->organization_sponsor); - list($valid, $invalid, $duplicate, $saved, $relationshipIds) = CRM_Contact_BAO_Relationship::create($params, $ids); + list($valid, $invalid, $duplicate, $saved, $relationshipIds) = CRM_Contact_BAO_Relationship::createMultiple($params, $ids); $this->assertEquals($valid, 1, 'In line ' . __LINE__); $this->assertEquals(empty($relationshipIds), FALSE, 'In line ' . __LINE__); @@ -307,7 +307,7 @@ DELETE FROM civicrm_contact_type 'contact_check' => array($this->organization_sponsor => 1), ); $ids = array('contact' => $this->indivi_student); - list($valid, $invalid, $duplicate, $saved, $relationshipIds) = CRM_Contact_BAO_Relationship::create($params, $ids); + list($valid, $invalid, $duplicate, $saved, $relationshipIds) = CRM_Contact_BAO_Relationship::createMultiple($params, $ids); $this->assertEquals($valid, 1, 'In line ' . __LINE__); $this->assertEquals(empty($relationshipIds), FALSE, 'In line ' . __LINE__); diff --git a/tests/phpunit/api/v3/AddressTest.php b/tests/phpunit/api/v3/AddressTest.php index 45c06e778f..1f00f7cd58 100644 --- a/tests/phpunit/api/v3/AddressTest.php +++ b/tests/phpunit/api/v3/AddressTest.php @@ -241,6 +241,7 @@ class api_v3_AddressTest extends CiviUnitTestCase { */ public function testGetAddressSort() { $create = $this->callAPISuccess('address', 'create', $this->_params); + $this->callAPISuccess('address', 'create', array_merge($this->_params, array('street_address' => 'yzy'))); $subfile = "AddressSort"; $description = "Demonstrates Use of sort filter"; $params = array( @@ -251,8 +252,8 @@ class api_v3_AddressTest extends CiviUnitTestCase { 'sequential' => 1, ); $result = $this->callAPIAndDocument('Address', 'Get', $params, __FUNCTION__, __FILE__, $description, $subfile); - $this->assertEquals(2, $result['count'], 'In line ' . __LINE__); - $this->assertEquals('Ambachtstraat 23', $result['values'][0]['street_address'], 'In line ' . __LINE__); + $this->assertEquals(2, $result['count']); + $this->assertEquals('Ambachtstraat 23', $result['values'][1]['street_address']); $this->callAPISuccess('address', 'delete', array('id' => $create['id'])); } @@ -277,7 +278,8 @@ class api_v3_AddressTest extends CiviUnitTestCase { */ public function testGetAddressLikeFail() { $create = $this->callAPISuccess('address', 'create', $this->_params); - $params = array('street_address' => array('LIKE' => "'%xy%'"), + $params = array( + 'street_address' => array('LIKE' => "'%xy%'"), 'sequential' => 1, ); $result = $this->callAPISuccess('Address', 'Get', ($params)); -- 2.25.1