Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CRM-20157, CRM-20195 Dedupe code cleanup #9907

Merged
merged 4 commits into from
Mar 8, 2017
Merged

CRM-20157, CRM-20195 Dedupe code cleanup #9907

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bf58823
CRM-20157 Code cleanup - remove unused & unuseful variables in dedupe…
eileenmcnaughton Feb 22, 2017
c4a43d7
CRM-20157 Code cleanup, remove duplicate variables.
eileenmcnaughton Feb 27, 2017
9c306ec
CRM-20157 code cleanup, more unused variables
eileenmcnaughton Feb 27, 2017
fcc3d8e
CRM-20195 remove deprecated fatals from dedupe merge form
eileenmcnaughton Feb 27, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
275 changes: 135 additions & 140 deletions CRM/Contact/Form/Merge.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,171 +64,166 @@ class CRM_Contact_Form_Merge extends CRM_Core_Form {
var $_qfZeroBug = 'e8cddb72-a257-11dc-b9cc-0016d3330ee9';

public function preProcess() {
if (!CRM_Core_Permission::check('merge duplicate contacts')) {
CRM_Core_Error::fatal(ts('You do not have access to this page'));
}

$cid = CRM_Utils_Request::retrieve('cid', 'Positive', $this, TRUE);
$oid = CRM_Utils_Request::retrieve('oid', 'Positive', $this, TRUE);
$flip = CRM_Utils_Request::retrieve('flip', 'Positive', $this, FALSE);

$this->_rgid = CRM_Utils_Request::retrieve('rgid', 'Positive', $this, FALSE);
$this->_gid = $gid = CRM_Utils_Request::retrieve('gid', 'Positive', $this, FALSE);
$this->_mergeId = CRM_Utils_Request::retrieve('mergeId', 'Positive', $this, FALSE);
$this->limit = CRM_Utils_Request::retrieve('limit', 'Positive', $this, FALSE);
$urlParams = "reset=1&rgid={$this->_rgid}&gid={$this->_gid}&limit=" . $this->limit;
try {

$this->bounceIfInvalid($cid, $oid);
$this->_cid = CRM_Utils_Request::retrieve('cid', 'Positive', $this, TRUE);
$this->_oid = CRM_Utils_Request::retrieve('oid', 'Positive', $this, TRUE);
$flip = CRM_Utils_Request::retrieve('flip', 'Positive', $this, FALSE);

$this->_contactType = civicrm_api3('Contact', 'getvalue', array('id' => $cid, 'return' => 'contact_type'));
$this->_rgid = CRM_Utils_Request::retrieve('rgid', 'Positive', $this, FALSE);
$this->_gid = $gid = CRM_Utils_Request::retrieve('gid', 'Positive', $this, FALSE);
$this->_mergeId = CRM_Utils_Request::retrieve('mergeId', 'Positive', $this, FALSE);
$this->limit = CRM_Utils_Request::retrieve('limit', 'Positive', $this, FALSE);
$urlParams = "reset=1&rgid={$this->_rgid}&gid={$this->_gid}&limit=" . $this->limit;

$browseUrl = CRM_Utils_System::url('civicrm/contact/dedupefind', $urlParams . '&action=browse');
$this->bounceIfInvalid($this->_cid, $this->_oid);

if (!$this->_rgid) {
// Unset browse URL as we have come from the search screen.
$browseUrl = '';
$this->_rgid = civicrm_api3('RuleGroup', 'getvalue', array(
'contact_type' => $this->_contactType,
'used' => 'Supervised',
'return' => 'id',
$this->_contactType = civicrm_api3('Contact', 'getvalue', array(
'id' => $this->_cid,
'return' => 'contact_type',
));
}
$this->assign('browseUrl', $browseUrl);
if ($browseUrl) {
CRM_Core_Session::singleton()->pushUserContext($browseUrl);
}

$cacheKey = CRM_Dedupe_Merger::getMergeCacheKeyString($this->_rgid, $gid);
$browseUrl = CRM_Utils_System::url('civicrm/contact/dedupefind', $urlParams . '&action=browse');

$join = CRM_Dedupe_Merger::getJoinOnDedupeTable();
$where = "de.id IS NULL";
if (!$this->_rgid) {
// Unset browse URL as we have come from the search screen.
$browseUrl = '';
$this->_rgid = civicrm_api3('RuleGroup', 'getvalue', array(
'contact_type' => $this->_contactType,
'used' => 'Supervised',
'return' => 'id',
));
}
$this->assign('browseUrl', $browseUrl);
if ($browseUrl) {
CRM_Core_Session::singleton()->pushUserContext($browseUrl);
}

$pos = CRM_Core_BAO_PrevNextCache::getPositions($cacheKey, $cid, $oid, $this->_mergeId, $join, $where, $flip);
$cacheKey = CRM_Dedupe_Merger::getMergeCacheKeyString($this->_rgid, $gid);

// get user info of main contact.
$config = CRM_Core_Config::singleton();
$config->doNotResetCache = 1;
$join = CRM_Dedupe_Merger::getJoinOnDedupeTable();
$where = "de.id IS NULL";

$viewUser = CRM_Core_Permission::check('access user profiles');
$mainUfId = CRM_Core_BAO_UFMatch::getUFId($cid);
$mainUser = NULL;
if ($mainUfId) {
// d6 compatible
if ($config->userSystem->is_drupal == '1') {
$mainUser = user_load($mainUfId);
}
elseif ($config->userFramework == 'Joomla') {
$mainUser = JFactory::getUser($mainUfId);
}
$pos = CRM_Core_BAO_PrevNextCache::getPositions($cacheKey, $this->_cid, $this->_oid, $this->_mergeId, $join, $where, $flip);

$this->assign('mainUfId', $mainUfId);
$this->assign('mainUfName', $mainUser ? $mainUser->name : NULL);
}
// get user info of main contact.
$config = CRM_Core_Config::singleton();
$config->doNotResetCache = 1;

$flipUrl = CRM_Utils_System::url('civicrm/contact/merge',
"reset=1&action=update&cid={$oid}&oid={$cid}&rgid={$this->_rgid}&gid={$gid}"
);
if (!$flip) {
$flipUrl .= '&flip=1';
}
$this->assign('flip', $flipUrl);

$this->prev = $this->next = NULL;
foreach (array(
'prev',
'next',
) as $position) {
if (!empty($pos[$position])) {
if ($pos[$position]['id1'] && $pos[$position]['id2']) {
$urlParams .= "&cid={$pos[$position]['id1']}&oid={$pos[$position]['id2']}&mergeId={$pos[$position]['mergeId']}&action=update";
$this->$position = CRM_Utils_System::url('civicrm/contact/merge', $urlParams);
$this->assign($position, $this->$position);
$mainUfId = CRM_Core_BAO_UFMatch::getUFId($this->_cid);
$mainUser = NULL;
if ($mainUfId) {
// d6 compatible
if ($config->userSystem->is_drupal == '1') {
$mainUser = user_load($mainUfId);
}
elseif ($config->userFramework == 'Joomla') {
$mainUser = JFactory::getUser($mainUfId);
}
}
}

// get user info of other contact.
$otherUfId = CRM_Core_BAO_UFMatch::getUFId($oid);
$otherUser = NULL;
$this->assign('mainUfId', $mainUfId);
$this->assign('mainUfName', $mainUser ? $mainUser->name : NULL);
}

if ($otherUfId) {
// d6 compatible
if ($config->userSystem->is_drupal == '1') {
$otherUser = user_load($otherUfId);
$flipUrl = CRM_Utils_System::url('civicrm/contact/merge',
"reset=1&action=update&cid={$this->_oid}&oid={$this->_cid}&rgid={$this->_rgid}&gid={$gid}"
);
if (!$flip) {
$flipUrl .= '&flip=1';
}
elseif ($config->userFramework == 'Joomla') {
$otherUser = JFactory::getUser($otherUfId);
$this->assign('flip', $flipUrl);

$this->prev = $this->next = NULL;
foreach (array(
'prev',
'next',
) as $position) {
if (!empty($pos[$position])) {
if ($pos[$position]['id1'] && $pos[$position]['id2']) {
$urlParams .= "&cid={$pos[$position]['id1']}&oid={$pos[$position]['id2']}&mergeId={$pos[$position]['mergeId']}&action=update";
$this->$position = CRM_Utils_System::url('civicrm/contact/merge', $urlParams);
$this->assign($position, $this->$position);
}
}
}

$this->assign('otherUfId', $otherUfId);
$this->assign('otherUfName', $otherUser ? $otherUser->name : NULL);
}

$cmsUser = ($mainUfId && $otherUfId) ? TRUE : FALSE;
$this->assign('user', $cmsUser);

$session = CRM_Core_Session::singleton();
// get user info of other contact.
$otherUfId = CRM_Core_BAO_UFMatch::getUFId($this->_oid);
$otherUser = NULL;

$rowsElementsAndInfo = CRM_Dedupe_Merger::getRowsElementsAndInfo($cid, $oid);
$main = $this->_mainDetails = $rowsElementsAndInfo['main_details'];
$other = $this->_otherDetails = $rowsElementsAndInfo['other_details'];

if ($main['contact_id'] != $cid) {
CRM_Core_Error::fatal(ts('The main contact record does not exist'));
}
if ($otherUfId) {
// d6 compatible
if ($config->userSystem->is_drupal == '1') {
$otherUser = user_load($otherUfId);
}
elseif ($config->userFramework == 'Joomla') {
$otherUser = JFactory::getUser($otherUfId);
}

if ($other['contact_id'] != $oid) {
CRM_Core_Error::fatal(ts('The other contact record does not exist'));
}
$this->assign('otherUfId', $otherUfId);
$this->assign('otherUfName', $otherUser ? $otherUser->name : NULL);
}

$this->assign('contact_type', $main['contact_type']);
$this->assign('main_name', $main['display_name']);
$this->assign('other_name', $other['display_name']);
$this->assign('main_cid', $main['contact_id']);
$this->assign('other_cid', $other['contact_id']);
$this->assign('rgid', $this->_rgid);

$this->_cid = $cid;
$this->_oid = $oid;

$this->addElement('checkbox', 'toggleSelect', NULL, NULL, array('class' => 'select-rows'));

$this->assign('mainLocBlock', json_encode($rowsElementsAndInfo['main_details']['location_blocks']));
$this->assign('locationBlockInfo', json_encode(CRM_Dedupe_Merger::getLocationBlockInfo()));
$this->assign('rows', $rowsElementsAndInfo['rows']);

// add elements
foreach ($rowsElementsAndInfo['elements'] as $element) {
// We could push this down to the getRowsElementsAndInfo function but it's
// already so overloaded - let's start moving towards doing form-things
// on the form.
if (substr($element[1], 0, 13) === 'move_location') {
$element[4] = array_merge(
(array) CRM_Utils_Array::value(4, $element, array()),
array('data-location' => substr($element[1], 14), 'data-is_location' => TRUE));
$cmsUser = ($mainUfId && $otherUfId) ? TRUE : FALSE;
$this->assign('user', $cmsUser);

$rowsElementsAndInfo = CRM_Dedupe_Merger::getRowsElementsAndInfo($this->_cid, $this->_oid);
$main = $this->_mainDetails = $rowsElementsAndInfo['main_details'];
$other = $this->_otherDetails = $rowsElementsAndInfo['other_details'];

$this->assign('contact_type', $main['contact_type']);
$this->assign('main_name', $main['display_name']);
$this->assign('other_name', $other['display_name']);
$this->assign('main_cid', $main['contact_id']);
$this->assign('other_cid', $other['contact_id']);
$this->assign('rgid', $this->_rgid);

$this->addElement('checkbox', 'toggleSelect', NULL, NULL, array('class' => 'select-rows'));

$this->assign('mainLocBlock', json_encode($rowsElementsAndInfo['main_details']['location_blocks']));
$this->assign('locationBlockInfo', json_encode(CRM_Dedupe_Merger::getLocationBlockInfo()));
$this->assign('rows', $rowsElementsAndInfo['rows']);

// add elements
foreach ($rowsElementsAndInfo['elements'] as $element) {
// We could push this down to the getRowsElementsAndInfo function but it's
// already so overloaded - let's start moving towards doing form-things
// on the form.
if (substr($element[1], 0, 13) === 'move_location') {
$element[4] = array_merge(
(array) CRM_Utils_Array::value(4, $element, array()),
array(
'data-location' => substr($element[1], 14),
'data-is_location' => TRUE,
));
}
if (substr($element[1], 0, 15) === 'location_blocks') {
// @todo We could add some data elements here to make jquery manipulation more straight-forward
// @todo consider enabling if it is an add & defaulting to true.
$element[4] = array_merge((array) CRM_Utils_Array::value(4, $element, array()), array('disabled' => TRUE));
}
$this->addElement($element[0],
$element[1],
array_key_exists('2', $element) ? $element[2] : NULL,
array_key_exists('3', $element) ? $element[3] : NULL,
array_key_exists('4', $element) ? $element[4] : NULL,
array_key_exists('5', $element) ? $element[5] : NULL
);
}
if (substr($element[1], 0, 15) === 'location_blocks') {
// @todo We could add some data elements here to make jquery manipulation more straight-forward
// @todo consider enabling if it is an add & defaulting to true.
$element[4] = array_merge((array) CRM_Utils_Array::value(4, $element, array()), array('disabled' => TRUE));

// add related table elements
foreach ($rowsElementsAndInfo['rel_table_elements'] as $relTableElement) {
$element = $this->addElement($relTableElement[0], $relTableElement[1]);
$element->setChecked(TRUE);
}
$this->addElement($element[0],
$element[1],
array_key_exists('2', $element) ? $element[2] : NULL,
array_key_exists('3', $element) ? $element[3] : NULL,
array_key_exists('4', $element) ? $element[4] : NULL,
array_key_exists('5', $element) ? $element[5] : NULL
);
}

// add related table elements
foreach ($rowsElementsAndInfo['rel_table_elements'] as $relTableElement) {
$element = $this->addElement($relTableElement[0], $relTableElement[1]);
$element->setChecked(TRUE);
$this->assign('rel_tables', $rowsElementsAndInfo['rel_tables']);
$this->assign('userContextURL', CRM_Core_Session::singleton()
->readUserContext());
}
catch (CRM_Core_Exception $e) {
CRM_Core_Error::statusBounce(ts($e->getMessage()));
}

$this->assign('rel_tables', $rowsElementsAndInfo['rel_tables']);
$this->assign('userContextURL', $session->readUserContext());
}

public function addRules() {
Expand Down
2 changes: 1 addition & 1 deletion CRM/Core/BAO/PrevNextCache.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ public static function retrieve($cacheKey, $join = NULL, $whereClause = NULL, $o

if (!empty($select)) {
$extraData = array();
foreach ($select as $dfield => $sfield) {
foreach ($select as $sfield) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

foreaches are faster if you specify the index - or so I hear?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did a quick google & didn't find anything to support that. Mostly I found for vs foreach - e.g https://coderwall.com/p/il1tog/php-for-vs-foreach-benchmark - but in general most of what I found was php 5.3 or 5.4- I expect it changes each php version

$extraData[$sfield] = $dao->$sfield;
}
$main[$count] = array(
Expand Down
1 change: 1 addition & 0 deletions CRM/Core/xml/Menu/Contact.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@
<path>civicrm/contact/merge</path>
<title>Merge Contact</title>
<page_callback>CRM_Contact_Form_Merge</page_callback>
<access_arguments>merge duplicate contacts</access_arguments>
</item>
<item>
<path>civicrm/contact/email</path>
Expand Down
17 changes: 7 additions & 10 deletions CRM/Dedupe/Merger.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1058,8 +1058,8 @@ public static function getRowsElementsAndInfo($mainId, $otherId, $checkPermissio
$qfZeroBug = 'e8cddb72-a257-11dc-b9cc-0016d3330ee9';
$fields = self::getMergeFieldsMetadata();

$main = self::getMergeContactDetails($mainId, 'main');
$other = self::getMergeContactDetails($otherId, 'main');
$main = self::getMergeContactDetails($mainId);
$other = self::getMergeContactDetails($otherId);
$specialValues['main'] = self::getSpecialValues($main);
$specialValues['other'] = self::getSpecialValues($other);

Expand Down Expand Up @@ -1165,7 +1165,7 @@ public static function getRowsElementsAndInfo($mainId, $otherId, $checkPermissio
$values = civicrm_api3($blockName, 'get', $searchParams);
if ($values['count']) {
$cnt = 0;
foreach ($values['values'] as $index => $value) {
foreach ($values['values'] as $value) {
$locations[$moniker][$blockName][$cnt] = $value;
// Fix address display
if ($blockName == 'address') {
Expand Down Expand Up @@ -1519,10 +1519,9 @@ public static function moveAllBelongings($mainId, $otherId, $migrationInfo, $che
CRM_Core_DAO::$_nullObject, NULL, -1
);
}
$cgTree = &$treeCache[$migrationInfo['main_details']['contact_type']];

$cFields = array();
foreach ($cgTree as $key => $group) {
foreach ($treeCache[$migrationInfo['main_details']['contact_type']] as $key => $group) {
if (!isset($group['fields'])) {
continue;
}
Expand Down Expand Up @@ -1993,13 +1992,12 @@ public static function getMergeFieldsMetadata() {
* Get the details of the contact to be merged.
*
* @param int $contact_id
* @param string $moniker
*
* @return array
*
* @throws CRM_Core_Exception
*/
public static function getMergeContactDetails($contact_id, $moniker) {
public static function getMergeContactDetails($contact_id) {
$params = array(
'contact_id' => $contact_id,
'version' => 3,
Expand All @@ -2009,9 +2007,8 @@ public static function getMergeContactDetails($contact_id, $moniker) {

// CRM-18480: Cancel the process if the contact is already deleted
if (isset($result['values'][$contact_id]['contact_is_deleted']) && !empty($result['values'][$contact_id]['contact_is_deleted'])) {
throw new CRM_Core_Exception(ts('Cannot merge because the \'%1\' contact (ID %2) has been deleted.', array(
1 => $moniker,
2 => $contact_id,
throw new CRM_Core_Exception(ts('Cannot merge because one contact (ID %1) has been deleted.', array(
1 => $contact_id,
)));
}

Expand Down