From 5ea06a7b545c605bb4666f6426d48b31745a9f1e Mon Sep 17 00:00:00 2001 From: eileen Date: Wed, 2 Mar 2016 15:31:40 +1300 Subject: [PATCH] CRM-18134 move api contact create function to the BAO --- CRM/Contact/Form/Merge.php | 14 ----------- CRM/Dedupe/Merger.php | 27 +++++++++++--------- api/v3/Contact.php | 34 ++++++++++++++----------- tests/phpunit/api/v3/ContactTest.php | 37 ++++++++++++++++++++++++++-- 4 files changed, 70 insertions(+), 42 deletions(-) diff --git a/CRM/Contact/Form/Merge.php b/CRM/Contact/Form/Merge.php index 3027767cc5..270275ab0b 100644 --- a/CRM/Contact/Form/Merge.php +++ b/CRM/Contact/Form/Merge.php @@ -321,20 +321,6 @@ class CRM_Contact_Form_Merge extends CRM_Core_Form { $message = ''; CRM_Core_Session::setStatus($message, ts('Contacts Merged'), 'success'); - //create activity for merge - //To do: this should be refactored into BAO layer at some point. - $messageActivity = ts('Contact ID %1 has been merged and deleted.', array(1 => $this->_oid)); - $activityParams = array( - 'subject' => $messageActivity, - 'source_contact_id' => $session->get('userID'), - 'target_contact_id' => $this->_cid, - 'activity_type_id' => CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_Activity', 'activity_type_id', 'Contact Merged'), - 'status_id' => 'Completed', - 'priority_id' => 'Normal', - 'activity_date_time' => date('YmdHis'), - ); - civicrm_api3('activity', 'create', $activityParams); - $url = CRM_Utils_System::url('civicrm/contact/view', "reset=1&cid={$this->_cid}"); if (!empty($formValues['_qf_Merge_submit'])) { $listParamsURL = "reset=1&action=update&rgid={$this->_rgid}"; diff --git a/CRM/Dedupe/Merger.php b/CRM/Dedupe/Merger.php index 6fba6f79b9..301e7f7059 100644 --- a/CRM/Dedupe/Merger.php +++ b/CRM/Dedupe/Merger.php @@ -1781,12 +1781,6 @@ INNER JOIN civicrm_membership membership2 ON membership1.membership_type_id = m CRM_Core_BAO_CustomValueTable::setValues($viewOnlyCustomFields); } - // **** Delete other contact & update prev-next caching - $otherParams = array( - 'contact_id' => $otherId, - 'id' => $otherId, - 'version' => 3, - ); if (CRM_Core_Permission::check('merge duplicate contacts') && CRM_Core_Permission::check('delete contacts') ) { @@ -1796,15 +1790,13 @@ INNER JOIN civicrm_membership membership2 ON membership1.membership_type_id = m CRM_Core_DAO::executeQuery($query); } - civicrm_api('contact', 'delete', $otherParams); + civicrm_api3('contact', 'delete', array('id' => $otherId)); CRM_Core_BAO_PrevNextCache::deleteItem($otherId); } // FIXME: else part - /* else { */ - - /* CRM_Core_Session::setStatus( ts('Do not have sufficient permission to delete duplicate contact.') ); */ - - /* } */ + // else { + // CRM_Core_Session::setStatus( ts('Do not have sufficient permission to delete duplicate contact.') ); + // } // CRM-15681 merge sub_types if ($other_sub_types = CRM_Utils_array::value('contact_sub_type', $migrationInfo['other_details'])) { @@ -1845,6 +1837,17 @@ INNER JOIN civicrm_membership membership2 ON membership1.membership_type_id = m CRM_Utils_Hook::post('merge', 'Contact', $mainId, CRM_Core_DAO::$_nullObject); + // Create activity for merge. + $messageActivity = ts('Contact ID %1 has been merged and deleted.', array(1 => $otherId)); + civicrm_api3('activity', 'create', array( + 'subject' => $messageActivity, + 'source_contact_id' => CRM_Core_Session::singleton()->getLoggedInContactID(), + 'target_contact_id' => $mainId, + 'activity_type_id' => 'Contact Merged', + 'status_id' => 'Completed', + 'priority_id' => 'Normal', + )); + return TRUE; } diff --git a/api/v3/Contact.php b/api/v3/Contact.php index 3dfe80f5a7..d27328284d 100644 --- a/api/v3/Contact.php +++ b/api/v3/Contact.php @@ -1021,39 +1021,45 @@ function _civicrm_api3_contact_deprecation() { * @throws CiviCRM_API3_Exception */ function civicrm_api3_contact_merge($params) { - $mode = CRM_Utils_Array::value('mode', $params, 'safe'); - $autoFlip = CRM_Utils_Array::value('auto_flip', $params, TRUE); - - $dupePairs = array( - array( - 'srcID' => CRM_Utils_Array::value('main_id', $params), - 'dstID' => CRM_Utils_Array::value('other_id', $params), - ), - ); - - if (($result = CRM_Dedupe_Merger::merge($dupePairs, array(), $mode, $autoFlip)) != FALSE) { + if (($result = CRM_Dedupe_Merger::merge(array( + array( + 'srcID' => $params['to_remove_id'], + 'dstID' => $params['to_keep_id'], + ), + ), array(), $params['mode'], $params['auto_flip'])) != FALSE) { return civicrm_api3_create_success($result, $params); } throw new CiviCRM_API3_Exception('Merge failed'); } /** - * Adjust metadata for contact_proximity api function. + * Adjust metadata for contact_merge api function. * * @param array $params */ function _civicrm_api3_contact_merge_spec(&$params) { - $params['main_id'] = array( + $params['to_remove_id'] = array( 'title' => 'ID of the contact to merge & remove', 'description' => ts('Wow - these 2 params are the logical reverse of what I expect - but what to do?'), 'api.required' => 1, 'type' => CRM_Utils_Type::T_INT, + 'api.aliases' => array('main_id'), ); - $params['other_id'] = array( + $params['to_keep_id'] = array( 'title' => 'ID of the contact to keep', 'description' => ts('Wow - these 2 params are the logical reverse of what I expect - but what to do?'), 'api.required' => 1, 'type' => CRM_Utils_Type::T_INT, + 'api.aliases' => array('other_id'), + ); + $params['auto_flip'] = array( + 'title' => 'Swap destination and source to retain lowest id?', + 'api.default' => TRUE, + ); + $params['mode'] = array( + // @todo need more detail on what this means. + 'title' => 'Dedupe mode', + 'api.default' => 'safe', ); } diff --git a/tests/phpunit/api/v3/ContactTest.php b/tests/phpunit/api/v3/ContactTest.php index 1ca8da8fd2..4e9599f022 100644 --- a/tests/phpunit/api/v3/ContactTest.php +++ b/tests/phpunit/api/v3/ContactTest.php @@ -81,6 +81,8 @@ class api_v3_ContactTest extends CiviUnitTestCase { 'civicrm_phone', 'civicrm_address', 'civicrm_acl_contact_cache', + 'civicrm_activity_contact', + 'civicrm_activity', ); $this->quickCleanup($tablesToTruncate, TRUE); @@ -2832,13 +2834,44 @@ class api_v3_ContactTest extends CiviUnitTestCase { /** * Test merging 2 contacts. + * + * Someone kindly bequethed us the legacy of mixed up use of main_id & other_id + * in the params for contact.merge api. + * + * This test protects that legacy. */ - public function testMerge() { + public function testMergeBizzareOldParams() { + $this->createLoggedInUser(); $otherContact = $this->callAPISuccess('contact', 'create', $this->_params); $mainContact = $this->callAPISuccess('contact', 'create', $this->_params); - $this->callAPISuccess('contact', 'merge', array('main_id' => $mainContact['id'], 'other_id' => $otherContact['id'])); + $this->callAPISuccess('contact', 'merge', array( + 'main_id' => $mainContact['id'], + 'other_id' => $otherContact['id'], + )); $contacts = $this->callAPISuccess('contact', 'get', $this->_params); $this->assertEquals($otherContact['id'], $contacts['id']); + } + + /** + * Test merging 2 contacts. + */ + public function testMerge() { + $this->createLoggedInUser(); + $otherContact = $this->callAPISuccess('contact', 'create', $this->_params); + $retainedContact = $this->callAPISuccess('contact', 'create', $this->_params); + $this->callAPISuccess('contact', 'merge', array( + 'to_keep_id' => $retainedContact['id'], + 'to_remove_id' => $otherContact['id'], + 'auto_flip' => FALSE, + )); + + $contacts = $this->callAPISuccess('contact', 'get', $this->_params); + $this->assertEquals($retainedContact['id'], $contacts['id']); + $activity = $this->callAPISuccess('Activity', 'getsingle', array( + 'target_contact_id' => $retainedContact['id'], + 'activity_type_id' => 'Contact Merged', + )); + $this->assertEquals(date('Y-m-d'), date('Y-m-d', strtotime($activity['activity_date_time']))); } -- 2.25.1