From bb22928bbdc1f2da6e1529a396905fbb37a8e30d Mon Sep 17 00:00:00 2001 From: eileen Date: Thu, 24 May 2018 16:44:07 +1200 Subject: [PATCH] Fix display of conflicts on duplicate screen. There are 2 bulk dedupe scenarios 1) dedupe selected 2) dedupe all In the case of the former, but not the latter, the display to which contacts are redirected should be filted by 'selected' Currently the selected filter is being applied whenever conflicts is true, resulting in an empty form in the latter case. This fix disambiguates the 2 concepts & attempts to rationalise & clarify related parameters in the flow. --- CRM/Contact/Page/AJAX.php | 2 +- CRM/Contact/Page/DedupeFind.php | 31 ++++++++++++++++++++--- CRM/Contact/Page/DedupeMerge.php | 18 +++++-------- CRM/Dedupe/Merger.php | 18 +++++++++++-- templates/CRM/Contact/Page/DedupeFind.tpl | 4 +-- 5 files changed, 53 insertions(+), 20 deletions(-) diff --git a/CRM/Contact/Page/AJAX.php b/CRM/Contact/Page/AJAX.php index 208d269204..937a63904f 100644 --- a/CRM/Contact/Page/AJAX.php +++ b/CRM/Contact/Page/AJAX.php @@ -651,7 +651,7 @@ LIMIT {$offset}, {$rowCount} $rgid = CRM_Utils_Request::retrieve('rgid', 'Positive'); $null = NULL; $criteria = CRM_Utils_Request::retrieve('criteria', 'Json', $null, FALSE, '{}'); - $selected = isset($_REQUEST['selected']) ? CRM_Utils_Type::escape($_REQUEST['selected'], 'Integer') : 0; + $selected = CRM_Utils_Request::retrieveValue('selected', 'Boolean'); if ($rowCount < 0) { $rowCount = 0; } diff --git a/CRM/Contact/Page/DedupeFind.php b/CRM/Contact/Page/DedupeFind.php index e96da5cde6..287f30ef11 100644 --- a/CRM/Contact/Page/DedupeFind.php +++ b/CRM/Contact/Page/DedupeFind.php @@ -36,6 +36,23 @@ class CRM_Contact_Page_DedupeFind extends CRM_Core_Page_Basic { protected $_mainContacts; protected $_gid; protected $action; + /** + * Only display selected. + * + * @var bool + */ + protected $selected; + + /** + * Get isSelected value. + * + * This needs to be an integer of 0 or 1 or NULL for no filter. + * + * @return bool|NULL + */ + public function isSelected() { + return ($this->selected === NULL) ? NULL : (int) $this->selected; + } /** * Get BAO Name. @@ -53,10 +70,18 @@ class CRM_Contact_Page_DedupeFind extends CRM_Core_Page_Basic { public function &links() { } + /** + * Initialize properties from input. + */ + protected function initialize() { + $this->selected = CRM_Utils_Request::retrieveValue('selected', 'Boolean'); + } + /** * Browse all rule groups. */ public function run() { + $this->initialize(); $gid = CRM_Utils_Request::retrieve('gid', 'Positive', $this, FALSE, 0); $action = CRM_Utils_Request::retrieve('action', 'String', $this, FALSE, 0); $context = CRM_Utils_Request::retrieve('context', 'String', $this); @@ -84,6 +109,7 @@ class CRM_Contact_Page_DedupeFind extends CRM_Core_Page_Basic { 'criteria' => $criteria, ); $this->assign('urlQuery', CRM_Utils_System::makeQueryString($urlQry)); + $this->assign('isSelected', $this->isSelected()); $criteria = json_decode($criteria, TRUE); if ($context == 'search') { @@ -142,9 +168,6 @@ class CRM_Contact_Page_DedupeFind extends CRM_Core_Page_Basic { $this->action = CRM_Core_Action::UPDATE; $urlQry['snippet'] = 4; - if ($isConflictMode) { - $urlQry['selected'] = 1; - } $this->assign('sourceUrl', CRM_Utils_System::url('civicrm/ajax/dedupefind', $urlQry, FALSE, NULL, FALSE)); @@ -160,7 +183,7 @@ class CRM_Contact_Page_DedupeFind extends CRM_Core_Page_Basic { CRM_Dedupe_Merger::resetMergeStats($cacheKeyString); } - $this->_mainContacts = CRM_Dedupe_Merger::getDuplicatePairs($rgid, $gid, !$isConflictMode, 0, $isConflictMode, '', $isConflictMode, $criteria, TRUE); + $this->_mainContacts = CRM_Dedupe_Merger::getDuplicatePairs($rgid, $gid, !$isConflictMode, 0, $this->isSelected(), '', $isConflictMode, $criteria, TRUE); if (empty($this->_mainContacts)) { if ($isConflictMode) { diff --git a/CRM/Contact/Page/DedupeMerge.php b/CRM/Contact/Page/DedupeMerge.php index 68ea12a3f0..5b0cd1e180 100644 --- a/CRM/Contact/Page/DedupeMerge.php +++ b/CRM/Contact/Page/DedupeMerge.php @@ -84,16 +84,9 @@ class CRM_Contact_Page_DedupeMerge extends CRM_Core_Page { )); $where = NULL; - if ($action == CRM_Core_Action::MAP) { - $where = "pn.is_selected = 1"; - $isSelected = 1; - } - else { - // else merge all (2) - $isSelected = 2; - } + $onlyProcessSelected = ($action == CRM_Core_Action::MAP) ? 1 : 0; - $total = CRM_Core_BAO_PrevNextCache::getCount($cacheKeyString, NULL, $where); + $total = CRM_Core_BAO_PrevNextCache::getCount($cacheKeyString, NULL, ($onlyProcessSelected ? "pn.is_selected = 1" : NULL)); if ($total <= 0) { // Nothing to do. CRM_Utils_System::redirect(CRM_Utils_System::url('civicrm/contact/dedupefind', $urlQry)); @@ -105,7 +98,7 @@ class CRM_Contact_Page_DedupeMerge extends CRM_Core_Page { for ($i = 1; $i <= ceil($total / self::BATCHLIMIT); $i++) { $task = new CRM_Queue_Task( array('CRM_Contact_Page_DedupeMerge', 'callBatchMerge'), - array($rgid, $gid, $mode, self::BATCHLIMIT, $isSelected, $criteria), + array($rgid, $gid, $mode, self::BATCHLIMIT, $onlyProcessSelected, $criteria), "Processed " . $i * self::BATCHLIMIT . " pair of duplicates out of " . $total ); @@ -115,6 +108,9 @@ class CRM_Contact_Page_DedupeMerge extends CRM_Core_Page { // Setup the Runner $urlQry['context'] = "conflicts"; + if ($onlyProcessSelected) { + $urlQry['selected'] = 1; + } $runner = new CRM_Queue_Runner(array( 'title' => ts('Merging Duplicates..'), 'queue' => $queue, @@ -140,7 +136,7 @@ class CRM_Contact_Page_DedupeMerge extends CRM_Core_Page { * @return int */ public static function callBatchMerge(CRM_Queue_TaskContext $ctx, $rgid, $gid, $mode = 'safe', $batchLimit, $isSelected, $criteria) { - CRM_Dedupe_Merger::batchMerge($rgid, $gid, $mode, $batchLimit, $isSelected, $criteria); + CRM_Dedupe_Merger::batchMerge($rgid, $gid, $mode, $batchLimit, $isSelected, $criteria, TRUE, FALSE); return CRM_Queue_Task::TASK_SUCCESS; } diff --git a/CRM/Dedupe/Merger.php b/CRM/Dedupe/Merger.php index 86b3ff0312..a148a56ee5 100644 --- a/CRM/Dedupe/Merger.php +++ b/CRM/Dedupe/Merger.php @@ -651,18 +651,32 @@ INNER JOIN civicrm_membership membership2 ON membership1.membership_type_id = m * mode does a force merge. * @param int $batchLimit number of merges to carry out in one batch. * @param int $isSelected if records with is_selected column needs to be processed. + * Note the option of '2' is only used in conjunction with $redirectForPerformance + * to determine when + * and the use of anything other than a boolean is being grandfathered out in favour of + * explicitly * * @param array $criteria * Criteria to use in the filter. * * @param bool $checkPermissions * Respect logged in user permissions. + * @param bool|NULL $reloadCacheIfEmpty + * If not set explicitly this is calculated but it is preferred that it be set + * per comments on isSelected above. * * @return array|bool */ - public static function batchMerge($rgid, $gid = NULL, $mode = 'safe', $batchLimit = 1, $isSelected = 2, $criteria = array(), $checkPermissions = TRUE) { + public static function batchMerge($rgid, $gid = NULL, $mode = 'safe', $batchLimit = 1, $isSelected = 2, $criteria = array(), $checkPermissions = TRUE, $reloadCacheIfEmpty = NULL) { $redirectForPerformance = ($batchLimit > 1) ? TRUE : FALSE; - $reloadCacheIfEmpty = (!$redirectForPerformance && $isSelected == 2); + + if (!isset($reloadCacheIfEmpty)) { + $reloadCacheIfEmpty = (!$redirectForPerformance && $isSelected == 2); + } + if ($isSelected !== 0 && $isSelected !== 1) { + // explicitly set to NULL if not 1 or 0 as part of grandfathering out the mystical '2' value. + $isSelected = NULL; + } $dupePairs = self::getDuplicatePairs($rgid, $gid, $reloadCacheIfEmpty, $batchLimit, $isSelected, '', ($mode == 'aggressive'), $criteria, $checkPermissions); $cacheParams = array( diff --git a/templates/CRM/Contact/Page/DedupeFind.tpl b/templates/CRM/Contact/Page/DedupeFind.tpl index 465acb4436..d5ac0f2fba 100644 --- a/templates/CRM/Contact/Page/DedupeFind.tpl +++ b/templates/CRM/Contact/Page/DedupeFind.tpl @@ -129,7 +129,7 @@ {ts}Safe Merge Selected Duplicates{/ts} {/if} - {capture assign=backURL}{crmURL p="civicrm/contact/dedupefind" q="`$urlQuery`&action=update" a=1}{/capture} + {capture assign=backURL}{crmURL p="civicrm/contact/dedupefind" q="`$urlQuery`&action=update&selected=0" a=1}{/capture} {ts}List All Duplicates{/ts} {else} {capture assign=backURL}{crmURL p="civicrm/contact/dedupefind" q="`$urlQuery`&action=renew" a=1}{/capture} @@ -162,7 +162,7 @@ (function($) { CRM.$('table#dupePairs').data({ "ajax": { - "url": {/literal}'{$sourceUrl}'{literal} + "url": {/literal}'{$sourceUrl}{if $isSelected}&selected=1{/if}'{literal} }, "retrieve": true, "processing": true, -- 2.25.1