CRM-18134 move api contact create function to the BAO
authoreileen <emcnaughton@wikimedia.org>
Wed, 2 Mar 2016 02:31:40 +0000 (15:31 +1300)
committereileen <emcnaughton@wikimedia.org>
Wed, 2 Mar 2016 03:14:19 +0000 (16:14 +1300)
CRM/Contact/Form/Merge.php
CRM/Dedupe/Merger.php
api/v3/Contact.php
tests/phpunit/api/v3/ContactTest.php

index 3027767cc5c1727619068b0b0d5a0938db5df1b3..270275ab0bcd9ba0242e90e0135420fedf437ad2 100644 (file)
@@ -321,20 +321,6 @@ class CRM_Contact_Form_Merge extends CRM_Core_Form {
     $message = '<ul><li>' . ts('%1 has been updated.', array(1 => $name)) . '</li><li>' . ts('Contact ID %1 has been deleted.', array(1 => $this->_oid)) . '</li></ul>';
     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}";
index 6fba6f79b9f14a3c06ae05083dd60a6fa1686475..301e7f70591bcec40768eb65320d20054a9eeac2 100644 (file)
@@ -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;
   }
 
index 3dfe80f5a74322308adfed114f792b3ed120a5ce..d27328284dcde80e6e9dca8c6bf29e5a5e499883 100644 (file)
@@ -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',
   );
 }
 
index 1ca8da8fd21f906c907939e0eb8155affcdf92d8..4e9599f02256de5ab2b2dccd186205b7749789dd 100644 (file)
@@ -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'])));
 
   }