From aefc291e1e76037e296b80fdacd0652dc38045aa Mon Sep 17 00:00:00 2001 From: eileen Date: Sat, 1 Jun 2019 18:42:10 +1200 Subject: [PATCH] Add Contact.get_merge_conflicts api THe goal here is to 1) improve testing 2) cleanup the code & define parameters where they are actually used rather than pass from pillar to post 3) expose to the api so we can be less coupled to the current form I thought about putting the api on the Dedupe entity but my thinking is that actions relating to individual pairs go on the contact entity & those relating to the batch find, cache, merge functionality go on the Dedupe entity. I'm doing another round of dedupe code cleanup at the moment... In conjunction with efforts to write an angular form which highlights what functionality a form needs to access & where the code is awful.... --- CRM/Dedupe/Merger.php | 22 ++++---- api/v3/Contact.php | 77 +++++++++++++++++++++++++--- tests/phpunit/api/v3/ContactTest.php | 12 ++++- 3 files changed, 95 insertions(+), 16 deletions(-) diff --git a/CRM/Dedupe/Merger.php b/CRM/Dedupe/Merger.php index d08d09e7f6..e6b07b9143 100644 --- a/CRM/Dedupe/Merger.php +++ b/CRM/Dedupe/Merger.php @@ -2109,15 +2109,7 @@ INNER JOIN civicrm_membership membership2 ON membership1.membership_type_id = m */ protected static function dedupePair(&$resultStats, &$deletedContacts, $mode, $checkPermissions, $mainId, $otherId, $cacheKeyString) { - // Generate var $migrationInfo. The variable structure is exactly same as - // $formValues submitted during a UI merge for a pair of contacts. - $rowsElementsAndInfo = CRM_Dedupe_Merger::getRowsElementsAndInfo($mainId, $otherId, $checkPermissions); - // add additional details that we might need to resolve conflicts - $rowsElementsAndInfo['migration_info']['main_details'] = &$rowsElementsAndInfo['main_details']; - $rowsElementsAndInfo['migration_info']['other_details'] = &$rowsElementsAndInfo['other_details']; - $rowsElementsAndInfo['migration_info']['rows'] = &$rowsElementsAndInfo['rows']; - $migrationInfo = $rowsElementsAndInfo['migration_info']; - // go ahead with merge if there is no conflict + $migrationInfo = []; $conflicts = []; if (!CRM_Dedupe_Merger::skipMerge($mainId, $otherId, $migrationInfo, $mode, $conflicts)) { CRM_Dedupe_Merger::moveAllBelongings($mainId, $otherId, $migrationInfo, $checkPermissions); @@ -2332,9 +2324,21 @@ INNER JOIN civicrm_membership membership2 ON membership1.membership_type_id = m * - Does a force merge otherwise (aggressive mode). * * @return array + * + * @throws \CRM_Core_Exception + * @throws \CiviCRM_API3_Exception */ public static function getConflicts(&$migrationInfo, $mainId, $otherId, $mode) { $conflicts = []; + // Generate var $migrationInfo. The variable structure is exactly same as + // $formValues submitted during a UI merge for a pair of contacts. + $rowsElementsAndInfo = CRM_Dedupe_Merger::getRowsElementsAndInfo($mainId, $otherId, FALSE); + // add additional details that we might need to resolve conflicts + $migrationInfo = $rowsElementsAndInfo['migration_info']; + $migrationInfo['main_details'] = &$rowsElementsAndInfo['main_details']; + $migrationInfo['other_details'] = &$rowsElementsAndInfo['other_details']; + $migrationInfo['rows'] = &$rowsElementsAndInfo['rows']; + // go ahead with merge if there is no conflict $originalMigrationInfo = $migrationInfo; foreach ($migrationInfo as $key => $val) { if ($val === "null") { diff --git a/api/v3/Contact.php b/api/v3/Contact.php index c493cad4c8..e7bddf9f0c 100644 --- a/api/v3/Contact.php +++ b/api/v3/Contact.php @@ -1213,22 +1213,87 @@ function civicrm_api3_contact_merge($params) { */ function _civicrm_api3_contact_merge_spec(&$params) { $params['to_remove_id'] = [ - '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?'), + 'title' => ts('ID of the contact to merge & remove'), + 'description' => ts('Wow - these 2 aliased params are the logical reverse of what I expect - but what to do?'), 'api.required' => 1, 'type' => CRM_Utils_Type::T_INT, 'api.aliases' => ['main_id'], ]; $params['to_keep_id'] = [ - '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?'), + 'title' => ts('ID of the contact to keep'), + 'description' => ts('Wow - these 2 aliased params are the logical reverse of what I expect - but what to do?'), 'api.required' => 1, 'type' => CRM_Utils_Type::T_INT, 'api.aliases' => ['other_id'], ]; $params['mode'] = [ - // @todo need more detail on what this means. - 'title' => 'Dedupe mode', + 'title' => ts('Dedupe mode'), + 'description' => ts("In 'safe' mode conflicts will result in no merge. In 'aggressive' mode the merge will still proceed (hook dependent)"), + 'api.default' => 'safe', + ]; +} + +/** + * Determines if given pair of contaacts have conflicts that would affect merging them. + * + * @param array $params + * Allowed array keys are: + * -int main_id: main contact id with whom merge has to happen + * -int other_id: duplicate contact which would be deleted after merge operation + * -string mode: "safe" skips the merge if there are no conflicts. Does a force merge otherwise. + * + * @return array + * API Result Array + * + * @throws \CRM_Core_Exception + * @throws \CiviCRM_API3_Exception + */ +function civicrm_api3_contact_get_merge_conflicts($params) { + $migrationInfo = []; + $result = CRM_Dedupe_Merger::getConflicts( + $migrationInfo, + $params['to_remove_id'], $params['to_keep_id'], + $params['mode'] + ); + $return = []; + foreach (array_keys($result) as $index) { + $return[str_replace('move_', '', $index)] = []; + } + $contacts = civicrm_api3('Contact', 'get', [ + 'id' => [ + 'IN' => [ + $params['to_keep_id'], + $params['to_remove_id'], + ], + ], + 'return' => array_keys($return), + ])['values']; + foreach (array_keys($return) as $fieldName) { + $return[$fieldName][$params['to_keep_id']] = CRM_Utils_Array::value($fieldName, $contacts[$params['to_keep_id']]); + $return[$fieldName][$params['to_remove_id']] = CRM_Utils_Array::value($fieldName, $contacts[$params['to_remove_id']]); + } + return civicrm_api3_create_success($return, $params); +} + +/** + * Adjust metadata for contact_merge api function. + * + * @param array $params + */ +function _civicrm_api3_contact_get_merge_conflicts_spec(&$params) { + $params['to_remove_id'] = [ + 'title' => ts('ID of the contact to merge & remove'), + 'api.required' => 1, + 'type' => CRM_Utils_Type::T_INT, + ]; + $params['to_keep_id'] = [ + 'title' => ts('ID of the contact to keep'), + 'api.required' => 1, + 'type' => CRM_Utils_Type::T_INT, + ]; + $params['mode'] = [ + 'title' => ts('Dedupe mode'), + 'description' => ts("'safe' or 'aggressive' - these modes map to the merge actions & may affect resolution done by hooks "), 'api.default' => 'safe', ]; } diff --git a/tests/phpunit/api/v3/ContactTest.php b/tests/phpunit/api/v3/ContactTest.php index b09fcd5ed1..4decfb56f9 100644 --- a/tests/phpunit/api/v3/ContactTest.php +++ b/tests/phpunit/api/v3/ContactTest.php @@ -1607,7 +1607,7 @@ class api_v3_ContactTest extends CiviUnitTestCase { * CRM-20421: This test make sure that inherited memberships are deleted upon merging organization. */ public function testMergeOrganizations() { - $organizationID1 = $this->organizationCreate(array(), 0); + $organizationID1 = $this->organizationCreate([], 0); $organizationID2 = $this->organizationCreate(array(), 1); $contact = $this->callAPISuccess('contact', 'create', array_merge($this->_params, array( 'employer_id' => $organizationID1, @@ -1650,6 +1650,16 @@ class api_v3_ContactTest extends CiviUnitTestCase { $this->assertEquals(0, $contactmembership["count"], "Contact membership must be deleted after merging organization without memberships."); } + /** + * Test the function that determines if 2 contacts have conflicts. + */ + public function testMergeGetConflicts() { + $contact1 = $this->individualCreate(); + $contact2 = $this->individualCreate(['first_name' => 'different']); + $conflicts = $this->callAPISuccess('Contact', 'get_merge_conflicts', ['to_keep_id' => $contact1, 'to_remove_id' => $contact2])['values']; + $this->assertEquals(['first_name' => [$contact1 => 'Anthony', $contact2 => 'different']], $conflicts); + } + private function createEmployerOfMembership() { $params = array( 'domain_id' => CRM_Core_Config::domainID(), -- 2.25.1