[MediaWiki-commits] [Gerrit] wikimedia...civicrm[master]: Towards CRM-20155 clean up form code in order to consolidate...

[jenkins-bot (Code Review)]

jenkins-bot has submitted this change and it was merged. ( 
https://gerrit.wikimedia.org/r/373173 )

Change subject: Towards CRM-20155 clean up form code in order to consolidate 
function use.
..


Towards CRM-20155 clean up form code in order to consolidate function use.

PR https://github.com/civicrm/civicrm-core/pull/10890

The code in the Find and Merge Duplicate Contacts form has mystified me for 
along time. As a step towards extracting
dedupe code into separate extension/s I have gone through & tidied up the 
finding of duplicates to re-use a function
used elsewhere rather than duplicate it on the form. In the process I tried, 
and failed, to come up with
a rationale for the duplicate form catching represented in setting form 
properties.

Note that I made the  parameter explicit. I am expecting this to be a URL 
paramter in the future
(currently deploying that to our site as such in the context of being able to 
find matches for specfic contacts.
I felt making it explicit now would aid in not missing instances of it when 
patching later

Change-Id: I694d71570db9f70406c30769b5ee9457305c25c8
---
M CRM/Contact/Form/Task/Merge.php
M CRM/Contact/Page/DedupeFind.php
M CRM/Dedupe/Merger.php
3 files changed, 41 insertions(+), 71 deletions(-)

Approvals:
  Mepps: Looks good to me, but someone else must approve
  jenkins-bot: Verified
  Ejegg: Looks good to me, approved



diff --git a/CRM/Contact/Form/Task/Merge.php b/CRM/Contact/Form/Task/Merge.php
index d0f0abd..9bbdf4e 100644
--- a/CRM/Contact/Form/Task/Merge.php
+++ b/CRM/Contact/Form/Task/Merge.php
@@ -33,7 +33,6 @@
 
 /**
  * This class provides the functionality to Merge contacts.
- *
  */
 class CRM_Contact_Form_Task_Merge extends CRM_Contact_Form_Task {
 
diff --git a/CRM/Contact/Page/DedupeFind.php b/CRM/Contact/Page/DedupeFind.php
index 012529f..5b09ef6 100644
--- a/CRM/Contact/Page/DedupeFind.php
+++ b/CRM/Contact/Page/DedupeFind.php
@@ -61,7 +61,19 @@
 $action = CRM_Utils_Request::retrieve('action', 'String', $this, FALSE, 0);
 $context = CRM_Utils_Request::retrieve('context', 'String', $this);
 $limit = CRM_Utils_Request::retrieve('limit', 'Integer', $this);
-$rgid = CRM_Utils_Request::retrieve('rgid', 'Positive');
+$rgid = CRM_Utils_Request::retrieve('rgid', 'Positive', $this);
+$cid = CRM_Utils_Request::retrieve('cid', 'Positive', $this, FALSE, 0);
+// Using a placeholder for criteria as it is intended to be able to pass 
this later.
+$criteria = array();
+$isConflictMode = ($context == 'conflicts');
+if ($cid) {
+  $this->_cid = $cid;
+}
+if ($gid) {
+  $this->_gid = $gid;
+}
+$this->_rgid = $rgid;
+
 $urlQry = array(
   'reset' => 1,
   'rgid' => $rgid,
@@ -78,14 +90,14 @@
 if ($action & CRM_Core_Action::RENEW) {
   // empty cache
   if ($rgid) {
-CRM_Core_BAO_PrevNextCache::deleteItem(NULL, 
CRM_Dedupe_Merger::getMergeCacheKeyString($rgid, $gid));
+CRM_Core_BAO_PrevNextCache::deleteItem(NULL, 
CRM_Dedupe_Merger::getMergeCacheKeyString($rgid, $gid, $criteria));
   }
   $urlQry['action'] = 'update';
   
CRM_Utils_System::redirect(CRM_Utils_System::url('civicrm/contact/dedupefind', 
$urlQry));
 }
 elseif ($action & CRM_Core_Action::MAP) {
   // do a batch merge if requested
-  $result = CRM_Dedupe_Merger::batchMerge($rgid, $gid, 'safe', 75);
+  $result = CRM_Dedupe_Merger::batchMerge($rgid, $gid, 'safe', 75, 2, 
$criteria);
 
   $skippedCount = CRM_Utils_Request::retrieve('skipped', 'Positive', 
$this, FALSE, 0);
   $skippedCount = $skippedCount + count($result['skipped']);
@@ -123,18 +135,17 @@
 if ($action & CRM_Core_Action::UPDATE ||
   $action & CRM_Core_Action::BROWSE
 ) {
-  $cid = CRM_Utils_Request::retrieve('cid', 'Positive', $this, FALSE, 0);
   $this->action = CRM_Core_Action::UPDATE;
 
   $urlQry['snippet'] = 4;
-  if ($context == 'conflicts') {
+  if ($isConflictMode) {
 $urlQry['selected'] = 1;
   }
 
   $this->assign('sourceUrl', 
CRM_Utils_System::url('civicrm/ajax/dedupefind', $urlQry, FALSE, NULL, FALSE));
 
   //reload from cache table
-  $cacheKeyString = CRM_Dedupe_Merger::getMergeCacheKeyString($rgid, $gid);
+  $cacheKeyString = CRM_Dedupe_Merger::getMergeCacheKeyString($rgid, $gid, 
$criteria);
 
   $stats = CRM_Dedupe_Merger::getMergeStatsMsg($cacheKeyString);
   if ($stats) {
@@ -142,80 +153,37 @@
 // reset so we not displaying same message again
 CRM_Dedupe_Merger::resetMergeStats($cacheKeyString);
   }
-  $join = CRM_Dedupe_Merger::getJoinOnDedupeTable();
-  $where = "de.id IS NULL";
-  if ($context == 'conflicts') {
-$where .= " AND pn.is_selected = 1";
-  }
-  $this->_mainContacts = 
CRM_Core_BAO_PrevNextCache::retrieve($cacheKeyString, $join, $where);
+
+  $this->_mainContacts = 

[MediaWiki-commits] [Gerrit] wikimedia...civicrm[master]: Towards CRM-20155 clean up form code in order to consolidate...

[Eileen (Code Review)]

Eileen has uploaded a new change for review. ( 
https://gerrit.wikimedia.org/r/373173 )

Change subject: Towards CRM-20155 clean up form code in order to consolidate 
function use.
..

Towards CRM-20155 clean up form code in order to consolidate function use.

PR https://github.com/civicrm/civicrm-core/pull/10890

The code in the Find and Merge Duplicate Contacts form has mystified me for 
along time. As a step towards extracting
dedupe code into separate extension/s I have gone through & tidied up the 
finding of duplicates to re-use a function
used elsewhere rather than duplicate it on the form. In the process I tried, 
and failed, to come up with
a rationale for the duplicate form catching represented in setting form 
properties.

Note that I made the  parameter explicit. I am expecting this to be a URL 
paramter in the future
(currently deploying that to our site as such in the context of being able to 
find matches for specfic contacts.
I felt making it explicit now would aid in not missing instances of it when 
patching later

Change-Id: I694d71570db9f70406c30769b5ee9457305c25c8
---
M CRM/Contact/Form/Task/Merge.php
M CRM/Contact/Page/DedupeFind.php
M CRM/Dedupe/Merger.php
3 files changed, 41 insertions(+), 71 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/wikimedia/fundraising/crm/civicrm 
refs/changes/73/373173/1

diff --git a/CRM/Contact/Form/Task/Merge.php b/CRM/Contact/Form/Task/Merge.php
index d0f0abd..9bbdf4e 100644
--- a/CRM/Contact/Form/Task/Merge.php
+++ b/CRM/Contact/Form/Task/Merge.php
@@ -33,7 +33,6 @@
 
 /**
  * This class provides the functionality to Merge contacts.
- *
  */
 class CRM_Contact_Form_Task_Merge extends CRM_Contact_Form_Task {
 
diff --git a/CRM/Contact/Page/DedupeFind.php b/CRM/Contact/Page/DedupeFind.php
index 012529f..5b09ef6 100644
--- a/CRM/Contact/Page/DedupeFind.php
+++ b/CRM/Contact/Page/DedupeFind.php
@@ -61,7 +61,19 @@
 $action = CRM_Utils_Request::retrieve('action', 'String', $this, FALSE, 0);
 $context = CRM_Utils_Request::retrieve('context', 'String', $this);
 $limit = CRM_Utils_Request::retrieve('limit', 'Integer', $this);
-$rgid = CRM_Utils_Request::retrieve('rgid', 'Positive');
+$rgid = CRM_Utils_Request::retrieve('rgid', 'Positive', $this);
+$cid = CRM_Utils_Request::retrieve('cid', 'Positive', $this, FALSE, 0);
+// Using a placeholder for criteria as it is intended to be able to pass 
this later.
+$criteria = array();
+$isConflictMode = ($context == 'conflicts');
+if ($cid) {
+  $this->_cid = $cid;
+}
+if ($gid) {
+  $this->_gid = $gid;
+}
+$this->_rgid = $rgid;
+
 $urlQry = array(
   'reset' => 1,
   'rgid' => $rgid,
@@ -78,14 +90,14 @@
 if ($action & CRM_Core_Action::RENEW) {
   // empty cache
   if ($rgid) {
-CRM_Core_BAO_PrevNextCache::deleteItem(NULL, 
CRM_Dedupe_Merger::getMergeCacheKeyString($rgid, $gid));
+CRM_Core_BAO_PrevNextCache::deleteItem(NULL, 
CRM_Dedupe_Merger::getMergeCacheKeyString($rgid, $gid, $criteria));
   }
   $urlQry['action'] = 'update';
   
CRM_Utils_System::redirect(CRM_Utils_System::url('civicrm/contact/dedupefind', 
$urlQry));
 }
 elseif ($action & CRM_Core_Action::MAP) {
   // do a batch merge if requested
-  $result = CRM_Dedupe_Merger::batchMerge($rgid, $gid, 'safe', 75);
+  $result = CRM_Dedupe_Merger::batchMerge($rgid, $gid, 'safe', 75, 2, 
$criteria);
 
   $skippedCount = CRM_Utils_Request::retrieve('skipped', 'Positive', 
$this, FALSE, 0);
   $skippedCount = $skippedCount + count($result['skipped']);
@@ -123,18 +135,17 @@
 if ($action & CRM_Core_Action::UPDATE ||
   $action & CRM_Core_Action::BROWSE
 ) {
-  $cid = CRM_Utils_Request::retrieve('cid', 'Positive', $this, FALSE, 0);
   $this->action = CRM_Core_Action::UPDATE;
 
   $urlQry['snippet'] = 4;
-  if ($context == 'conflicts') {
+  if ($isConflictMode) {
 $urlQry['selected'] = 1;
   }
 
   $this->assign('sourceUrl', 
CRM_Utils_System::url('civicrm/ajax/dedupefind', $urlQry, FALSE, NULL, FALSE));
 
   //reload from cache table
-  $cacheKeyString = CRM_Dedupe_Merger::getMergeCacheKeyString($rgid, $gid);
+  $cacheKeyString = CRM_Dedupe_Merger::getMergeCacheKeyString($rgid, $gid, 
$criteria);
 
   $stats = CRM_Dedupe_Merger::getMergeStatsMsg($cacheKeyString);
   if ($stats) {
@@ -142,80 +153,37 @@
 // reset so we not displaying same message again
 CRM_Dedupe_Merger::resetMergeStats($cacheKeyString);
   }
-  $join = CRM_Dedupe_Merger::getJoinOnDedupeTable();
-  $where = "de.id IS NULL";
-  if ($context == 'conflicts') {
-$where .= " AND pn.is_selected = 1";
-  }
-  $this->_mainContacts = 
CRM_Core_BAO_PrevNextCache::retrieve($cacheKeyString, $join, $where);
+
+  $this->_mainContacts = CRM_Dedupe_Merger::getDuplicatePairs($rgid,