Merge pull request #8689 from eileenmcnaughton/dupe_pairs

CRM-19035 fix mixing of dst & srcid by eliminating autoflip
civicrm · Jul 13, 2016 · 323b062 · 323b062
2 parents e5dbcc1 + 063ffcb
commit 323b062
Show file tree

Hide file tree

Showing 9 changed files with 228 additions and 155 deletions.
diff --git a/CRM/Contact/Page/AJAX.php b/CRM/Contact/Page/AJAX.php
@@ -676,14 +676,14 @@ public static function getDedupes() {
 
     $nextParamKey = 3;
     $mappings = array(
-      'src' => 'cc1.display_name',
-      'dst' => 'cc2.display_name',
-      'src_email' => 'ce1.email',
-      'dst_email' => 'ce2.email',
-      'src_postcode' => 'ca1.postal_code',
-      'dst_postcode' => 'ca2.postal_code',
-      'src_street' => 'ca1.street',
-      'dst_street' => 'ca2.street',
+      'dst' => 'cc1.display_name',
+      'src' => 'cc2.display_name',
+      'dst_email' => 'ce1.email',
+      'src_email' => 'ce2.email',
+      'dst_postcode' => 'ca1.postal_code',
+      'src_postcode' => 'ca2.postal_code',
+      'dst_street' => 'ca1.street',
+      'src_street' => 'ca2.street',
     );
 
     foreach ($mappings as $key => $dbName) {
@@ -710,18 +710,18 @@ public static function getDedupes() {
     $join .= CRM_Dedupe_Merger::getJoinOnDedupeTable();
 
     $select = array(
-      'cc1.contact_type'     => 'src_contact_type',
-      'cc1.display_name'     => 'src_display_name',
-      'cc1.contact_sub_type' => 'src_contact_sub_type',
-      'cc2.contact_type'     => 'dst_contact_type',
-      'cc2.display_name'     => 'dst_display_name',
-      'cc2.contact_sub_type' => 'dst_contact_sub_type',
-      'ce1.email'            => 'src_email',
-      'ce2.email'            => 'dst_email',
-      'ca1.postal_code'      => 'src_postcode',
-      'ca2.postal_code'      => 'dst_postcode',
-      'ca1.street_address'   => 'src_street',
-      'ca2.street_address'   => 'dst_street',
+      'cc1.contact_type'     => 'dst_contact_type',
+      'cc1.display_name'     => 'dst_display_name',
+      'cc1.contact_sub_type' => 'dst_contact_sub_type',
+      'cc2.contact_type'     => 'src_contact_type',
+      'cc2.display_name'     => 'src_display_name',
+      'cc2.contact_sub_type' => 'src_contact_sub_type',
+      'ce1.email'            => 'dst_email',
+      'ce2.email'            => 'src_email',
+      'ca1.postal_code'      => 'dst_postcode',
+      'ca2.postal_code'      => 'src_postcode',
+      'ca1.street_address'   => 'dst_street',
+      'ca2.street_address'   => 'src_street',
     );
 
     if ($select) {
@@ -745,38 +745,39 @@ public static function getDedupes() {
     if (!empty($columnDetails)) {
       switch ($columnDetails['data']) {
         case 'src':
-          $orderByClause = " ORDER BY cc1.display_name {$dir}";
+          $orderByClause = " ORDER BY cc2.display_name {$dir}";
           break;
 
         case 'src_email':
-          $orderByClause = " ORDER BY ce1.email {$dir}";
+          $orderByClause = " ORDER BY ce2.email {$dir}";
           break;
 
         case 'src_street':
-          $orderByClause = " ORDER BY ca1.street_address {$dir}";
+          $orderByClause = " ORDER BY ca2.street_address {$dir}";
           break;
 
         case 'src_postcode':
-          $orderByClause = " ORDER BY ca1.postal_code {$dir}";
+          $orderByClause = " ORDER BY ca2.postal_code {$dir}";
           break;
 
         case 'dst':
-          $orderByClause = " ORDER BY cc2.display_name {$dir}";
+          $orderByClause = " ORDER BY cc1.display_name {$dir}";
           break;
 
         case 'dst_email':
-          $orderByClause = " ORDER BY ce2.email {$dir}";
+          $orderByClause = " ORDER BY ce1.email {$dir}";
           break;
 
         case 'dst_street':
-          $orderByClause = " ORDER BY ca2.street_address {$dir}";
+          $orderByClause = " ORDER BY ca1.street_address {$dir}";
           break;
 
         case 'dst_postcode':
-          $orderByClause = " ORDER BY ca2.postal_code {$dir}";
+          $orderByClause = " ORDER BY ca1.postal_code {$dir}";
           break;
 
         default:
+          $orderByClause = " ORDER BY cc1.display_name ASC";
           break;
       }
     }
@@ -786,29 +787,29 @@ public static function getDedupes() {
 
     $count = 0;
     foreach ($dupePairs as $key => $pairInfo) {
-      $pair =& $pairInfo['data'];
+      $pair = $pairInfo['data'];
       $srcContactSubType  = CRM_Utils_Array::value('src_contact_sub_type', $pairInfo);
       $dstContactSubType  = CRM_Utils_Array::value('dst_contact_sub_type', $pairInfo);
       $srcTypeImage = CRM_Contact_BAO_Contact_Utils::getImage($srcContactSubType ?
         $srcContactSubType : $pairInfo['src_contact_type'],
         FALSE,
-        $pairInfo['entity_id1']
+        $pairInfo['entity_id2']
       );
       $dstTypeImage = CRM_Contact_BAO_Contact_Utils::getImage($dstContactSubType ?
         $dstContactSubType : $pairInfo['dst_contact_type'],
         FALSE,
-        $pairInfo['entity_id2']
+        $pairInfo['entity_id1']
       );
 
       $searchRows[$count]['is_selected'] = $pairInfo['is_selected'];
       $searchRows[$count]['is_selected_input'] = "<input type='checkbox' class='crm-dedupe-select' name='pnid_{$pairInfo['prevnext_id']}' value='{$pairInfo['is_selected']}' onclick='toggleDedupeSelect(this)'>";
       $searchRows[$count]['src_image'] = $srcTypeImage;
-      $searchRows[$count]['src'] = CRM_Utils_System::href($pair['srcName'], 'civicrm/contact/view', "reset=1&cid={$pairInfo['entity_id1']}");
+      $searchRows[$count]['src'] = CRM_Utils_System::href($pair['srcName'], 'civicrm/contact/view', "reset=1&cid={$pairInfo['entity_id2']}");
       $searchRows[$count]['src_email'] = CRM_Utils_Array::value('src_email', $pairInfo);
       $searchRows[$count]['src_street'] = CRM_Utils_Array::value('src_street', $pairInfo);
       $searchRows[$count]['src_postcode'] = CRM_Utils_Array::value('src_postcode', $pairInfo);
       $searchRows[$count]['dst_image'] = $dstTypeImage;
-      $searchRows[$count]['dst'] = CRM_Utils_System::href($pair['dstName'], 'civicrm/contact/view', "reset=1&cid={$pairInfo['entity_id2']}");
+      $searchRows[$count]['dst'] = CRM_Utils_System::href($pair['dstName'], 'civicrm/contact/view', "reset=1&cid={$pairInfo['entity_id1']}");
       $searchRows[$count]['dst_email'] = CRM_Utils_Array::value('dst_email', $pairInfo);
       $searchRows[$count]['dst_street'] = CRM_Utils_Array::value('dst_street', $pairInfo);
       $searchRows[$count]['dst_postcode'] = CRM_Utils_Array::value('dst_postcode', $pairInfo);

diff --git a/CRM/Contact/Page/DedupeFind.php b/CRM/Contact/Page/DedupeFind.php
@@ -207,66 +207,8 @@ public function run() {
           CRM_Utils_System::redirect($url);
         }
         else {
-          $cids = array();
-          foreach ($foundDupes as $dupe) {
-            $cids[$dupe[0]] = 1;
-            $cids[$dupe[1]] = 1;
-          }
-          $cidString = implode(', ', array_keys($cids));
-          $sql = "SELECT id, display_name FROM civicrm_contact WHERE id IN ($cidString) ORDER BY sort_name";
-          $dao = new CRM_Core_DAO();
-          $dao->query($sql);
-          $displayNames = array();
-          while ($dao->fetch()) {
-            $displayNames[$dao->id] = $dao->display_name;
-          }
-
-          // FIXME: sort the contacts; $displayName
-          // is already sort_name-sorted, so use that
-          // (also, consider sorting by dupe count first)
-          // lobo - change the sort to by threshold value
-          // so the more likely dupes are sorted first
-          $userId = CRM_Core_Session::singleton()->getLoggedInContactID();
-          $mainContacts = $permission = array();
-
-          foreach ($foundDupes as $dupes) {
-            $srcID = $dupes[0];
-            $dstID = $dupes[1];
-            if ($dstID == $userId) {
-              $srcID = $dupes[1];
-              $dstID = $dupes[0];
-            }
+          $mainContacts = CRM_Dedupe_Finder::parseAndStoreDupePairs($foundDupes, $cacheKeyString);
 
-            /***
-             * Eliminate this since it introduces 3 queries PER merge row
-             * and hence is very expensive
-             * CRM-8822
-             * if ( !array_key_exists( $srcID, $permission ) ) {
-             * $permission[$srcID] = CRM_Contact_BAO_Contact_Permission::allow( $srcID, CRM_Core_Permission::EDIT );
-             * }
-             * if ( !array_key_exists( $dstID, $permission ) ) {
-             * $permission[$dstID] = CRM_Contact_BAO_Contact_Permission::allow( $dstID, CRM_Core_Permission::EDIT );
-             * }
-             *
-             * $canMerge = ( $permission[$dstID] && $permission[$srcID] );
-             *
-             */
-
-            // we'll do permission checking during the merge process
-            $canMerge = TRUE;
-
-            $mainContacts[] = $row = array(
-              'srcID' => $srcID,
-              'srcName' => $displayNames[$srcID],
-              'dstID' => $dstID,
-              'dstName' => $displayNames[$dstID],
-              'weight' => $dupes[2],
-              'canMerge' => $canMerge,
-            );
-
-            $data = CRM_Core_DAO::escapeString(serialize($row));
-            $values[] = " ( 'civicrm_contact', $srcID, $dstID, '$cacheKeyString', '$data' ) ";
-          }
           if ($cid) {
             $this->_cid = $cid;
           }
@@ -276,7 +218,6 @@ public function run() {
           $this->_rgid = $rgid;
           $this->_mainContacts = $mainContacts;
 
-          CRM_Core_BAO_PrevNextCache::setItem($values);
           $session = CRM_Core_Session::singleton();
           if ($this->_cid) {
             $session->pushUserContext(CRM_Utils_System::url('civicrm/contact/deduperules',

diff --git a/CRM/Core/BAO/PrevNextCache.php b/CRM/Core/BAO/PrevNextCache.php
@@ -384,43 +384,7 @@ public static function refillCache($rgid, $gid, $cacheKeyString, $criteria, $che
     }
 
     if (!empty($foundDupes)) {
-      $cids = $displayNames = $values = array();
-      foreach ($foundDupes as $dupe) {
-        $cids[$dupe[0]] = 1;
-        $cids[$dupe[1]] = 1;
-      }
-      $cidString = implode(', ', array_keys($cids));
-      $sql = "SELECT id, display_name FROM civicrm_contact WHERE id IN ($cidString) ORDER BY sort_name";
-      $dao = new CRM_Core_DAO();
-      $dao->query($sql);
-      while ($dao->fetch()) {
-        $displayNames[$dao->id] = $dao->display_name;
-      }
-
-      $session = CRM_Core_Session::singleton();
-      $userId = $session->get('userID');
-
-      foreach ($foundDupes as $dupes) {
-        $srcID = $dupes[0];
-        $dstID = $dupes[1];
-        if ($dstID == $userId) {
-          $srcID = $dupes[1];
-          $dstID = $dupes[0];
-        }
-
-        $row = array(
-          'srcID' => $srcID,
-          'srcName' => $displayNames[$srcID],
-          'dstID' => $dstID,
-          'dstName' => $displayNames[$dstID],
-          'weight' => $dupes[2],
-          'canMerge' => TRUE,
-        );
-
-        $data = CRM_Core_DAO::escapeString(serialize($row));
-        $values[] = " ( 'civicrm_contact', $srcID, $dstID, '$cacheKeyString', '$data' ) ";
-      }
-      self::setItem($values);
+      CRM_Dedupe_Finder::parseAndStoreDupePairs($foundDupes, $cacheKeyString);
     }
   }
 
@@ -627,8 +591,8 @@ public static function flipPair(array $prevNextId, $onlySelected) {
           $data['dst' . $key] = $originalData['src' . $key];
         }
         $dao->data = serialize($data);
-        $dao->entity_id1 = $data['srcID'];
-        $dao->entity_id2 = $data['dstID'];
+        $dao->entity_id1 = $data['dstID'];
+        $dao->entity_id2 = $data['srcID'];
         $dao->save();
       }
     }

diff --git a/CRM/Dedupe/BAO/RuleGroup.php b/CRM/Dedupe/BAO/RuleGroup.php
@@ -378,7 +378,8 @@ public function thresholdQuery($checkPermission = TRUE) {
         list($this->_aclFrom, $this->_aclWhere) = CRM_Contact_BAO_Contact_Permission::cacheClause(array('c1', 'c2'));
         $this->_aclWhere = $this->_aclWhere ? "AND {$this->_aclWhere}" : '';
       }
-      $query = "SELECT dedupe.id1, dedupe.id2, dedupe.weight
+      $query = "SELECT IF(dedupe.id1 < dedupe.id2, dedupe.id1, dedupe.id2) as id1,
+                IF(dedupe.id1 < dedupe.id2, dedupe.id2, dedupe.id1) as id2, dedupe.weight
                 FROM dedupe JOIN civicrm_contact c1 ON dedupe.id1 = c1.id
                             JOIN civicrm_contact c2 ON dedupe.id2 = c2.id {$this->_aclFrom}
                        LEFT JOIN civicrm_dedupe_exception exc ON dedupe.id1 = exc.contact_id1 AND dedupe.id2 = exc.contact_id2

diff --git a/CRM/Dedupe/Finder.php b/CRM/Dedupe/Finder.php
@@ -348,4 +348,61 @@ public static function formatParams($fields, $ctype) {
     return $params;
   }
 
+  /**
+   * Parse duplicate pairs into a standardised array and store in the prev_next_cache.
+   *
+   * @param array $foundDupes
+   * @param string $cacheKeyString
+   *
+   * @return array Dupe pairs with the keys
+   *   Dupe pairs with the keys
+   *   -srcID
+   *   -srcName
+   *   -dstID
+   *   -dstName
+   *   -weight
+   *   -canMerge
+   *
+   * @throws CRM_Core_Exception
+   */
+  public static function parseAndStoreDupePairs($foundDupes, $cacheKeyString) {
+    $cids = array();
+    foreach ($foundDupes as $dupe) {
+      $cids[$dupe[0]] = 1;
+      $cids[$dupe[1]] = 1;
+    }
+    $cidString = implode(', ', array_keys($cids));
+
+    $dao = CRM_Core_DAO::executeQuery("SELECT id, display_name FROM civicrm_contact WHERE id IN ($cidString) ORDER BY sort_name");
+    $displayNames = array();
+    while ($dao->fetch()) {
+      $displayNames[$dao->id] = $dao->display_name;
+    }
+
+    $userId = CRM_Core_Session::singleton()->getLoggedInContactID();
+    foreach ($foundDupes as $dupes) {
+      $srcID = $dupes[1];
+      $dstID = $dupes[0];
+      // The logged in user should never be the src (ie. the contact to be removed).
+      if ($srcID == $userId) {
+        $srcID = $dstID;
+        $dstID = $userId;
+      }
+
+      $mainContacts[] = $row = array(
+        'dstID' => $dstID,
+        'dstName' => $displayNames[$dstID],
+        'srcID' => $srcID,
+        'srcName' => $displayNames[$srcID],
+        'weight' => $dupes[2],
+        'canMerge' => TRUE,
+      );
+
+      $data = CRM_Core_DAO::escapeString(serialize($row));
+      $values[] = " ( 'civicrm_contact', $dstID, $srcID, '$cacheKeyString', '$data' ) ";
+    }
+    CRM_Core_BAO_PrevNextCache::setItem($values);
+    return $mainContacts;
+  }
+
 }
diff --git a/CRM/Dedupe/Merger.php b/CRM/Dedupe/Merger.php
@@ -706,7 +706,7 @@ public static function getMergeStatsMsg($cacheKeyString) {
       $msg = "{$stats['merged']} " . ts('Contact(s) were merged.');
     }
     if (!empty($stats['skipped'])) {
-      $msg .= $stats['skipped'] . ts('Contact(s) were skipped.');
+      $msg .= $stats['skipped'] . ts(' Contact(s) were skipped.');
     }
     return $msg;
   }
@@ -755,12 +755,7 @@ public static function merge($dupePairs = array(), $cacheParams = array(), $mode
         CRM_Utils_Hook::merge('flip', $dupes, $dupes['dstID'], $dupes['srcID']);
         $mainId = $dupes['dstID'];
         $otherId = $dupes['srcID'];
-        $isAutoFlip = CRM_Utils_Array::value('auto_flip', $dupes, $autoFlip);
-        // if we can, make sure that $mainId is the one with lower id number
-        if ($isAutoFlip && ($mainId > $otherId)) {
-          $mainId = $dupes['srcID'];
-          $otherId = $dupes['dstID'];
-        }
+
         if (!$mainId || !$otherId) {
           // return error
           return FALSE;

diff --git a/templates/CRM/Contact/Page/DedupeFind.tpl b/templates/CRM/Contact/Page/DedupeFind.tpl
@@ -98,16 +98,16 @@
     <thead>
       <tr class="columnheader">
         <th data-data="is_selected_input" class="crm-dedupe-selection"><input type="checkbox" value="0" name="pnid_all" class="crm-dedupe-select-all"></th>
-        <th data-data="src_image"    class="crm-empty">&nbsp;</th>
-        <th data-data="src"          class="crm-contact">{ts}Contact{/ts} 1</th>
         <th data-data="dst_image"    class="crm-empty">&nbsp;</th>
-        <th data-data="dst"          class="crm-contact-duplicate">{ts}Contact{/ts} 2 ({ts}Duplicate{/ts})</th>
-        <th data-data="src_email"    class="crm-contact">{ts}Email{/ts} 1</th>
-        <th data-data="dst_email"    class="crm-contact-duplicate">{ts}Email{/ts} 2 ({ts}Duplicate{/ts})</th>
-        <th data-data="src_street"   class="crm-contact">{ts}Street Address{/ts} 1</th>
-        <th data-data="dst_street"   class="crm-contact-duplicate">{ts}Street Address{/ts} 2 ({ts}Duplicate{/ts})</th>
-        <th data-data="src_postcode" class="crm-contact">{ts}Postcode{/ts} 1</th>
-        <th data-data="dst_postcode" class="crm-contact-duplicate">{ts}Postcode{/ts} 2 ({ts}Duplicate{/ts})</th>
+        <th data-data="dst"          class="crm-contact">{ts}Contact{/ts} 1</th>
+        <th data-data="src_image"    class="crm-empty">&nbsp;</th>
+        <th data-data="src"          class="crm-contact-duplicate">{ts}Contact{/ts} 2 ({ts}Duplicate{/ts})</th>
+        <th data-data="dst_email"    class="crm-contact">{ts}Email{/ts} 1</th>
+        <th data-data="src_email"    class="crm-contact-duplicate">{ts}Email{/ts} 2 ({ts}Duplicate{/ts})</th>
+        <th data-data="dst_street"   class="crm-contact">{ts}Street Address{/ts} 1</th>
+        <th data-data="src_street"   class="crm-contact-duplicate">{ts}Street Address{/ts} 2 ({ts}Duplicate{/ts})</th>
+        <th data-data="dst_postcode" class="crm-contact">{ts}Postcode{/ts} 1</th>
+        <th data-data="src_postcode" class="crm-contact-duplicate">{ts}Postcode{/ts} 2 ({ts}Duplicate{/ts})</th>
         <th data-data="conflicts"    class="crm-contact-conflicts">{ts}Conflicts{/ts}</th>
         <th data-data="weight"       class="crm-threshold">{ts}Threshold{/ts}</th>
         <th data-data="actions"      class="crm-empty">&nbsp;</th>