-
-
Notifications
You must be signed in to change notification settings - Fork 825
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.
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
Re-instate Dedupe limit functionality & fix select toggle functionality #12305
Conversation
It turned out the toggleDuplicates was not working when criteria was set as the validation rule didn't work. Passing around cacheKey is easier to validate and we know the cache willbe created at the point of toggle. Use cacheKey instead in url
(Standard links)
|
@@ -324,8 +324,7 @@ | |||
var is_selected = CRM.$('.crm-dedupe-select-all').prop('checked') ? 1 : 0; | |||
} | |||
|
|||
var criteria = {/literal}'{$criteria|escape}'{literal}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This escape wasn't working - hence the switch to cachekey string in this function
@@ -2035,7 +2036,7 @@ public static function getDuplicatePairs($rule_group_id, $group_id, $reloadCache | |||
*/ | |||
public static function getMergeCacheKeyString($rule_group_id, $group_id, $criteria = array(), $checkPermissions = TRUE) { | |||
$contactType = CRM_Dedupe_BAO_RuleGroup::getContactTypeForRuleGroup($rule_group_id); | |||
$cacheKeyString = "merge {$contactType}"; | |||
$cacheKeyString = "merge_{$contactType}"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add _ since we are now passing in the url & want it to be Alphanumeric
@@ -181,7 +181,7 @@ public static function dupesByParams( | |||
* array of (cid1, cid2, weight) dupe triples | |||
*/ | |||
public static function dupesInGroup($rgid, $gid, $searchLimit = 0) { | |||
$cids = array_keys(CRM_Contact_BAO_Group::getMember($gid, $searchLimit)); | |||
$cids = array_keys(CRM_Contact_BAO_Group::getMember($gid, TRUE, $searchLimit)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was just an OOPS - was passing $searchLimit as param 2 not param 3!
@@ -183,7 +182,7 @@ public function run() { | |||
CRM_Dedupe_Merger::resetMergeStats($cacheKeyString); | |||
} | |||
|
|||
$this->_mainContacts = CRM_Dedupe_Merger::getDuplicatePairs($rgid, $gid, !$isConflictMode, 0, $this->isSelected(), '', $isConflictMode, $criteria, TRUE); | |||
$this->_mainContacts = CRM_Dedupe_Merger::getDuplicatePairs($rgid, $gid, !$isConflictMode, 0, $this->isSelected(), '', $isConflictMode, $criteria, TRUE, $limit); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pass limit through
$pnid = $_REQUEST['pnid']; | ||
$isSelected = CRM_Utils_Type::escape($_REQUEST['is_selected'], 'Boolean'); | ||
$criteria = CRM_Utils_Request::retrieve('criteria', 'Json', $null, FALSE, '{}'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this function wasn't working when criteria was passed because the smarty layer escaping was leaving it empty & the cachekey couldn't be calculated. Turns out the criteria is really only needed to calculate the cacheKey so instead just pass that & simplify. Note the very last line on this PR's changes is where we change what is passed into this PR
@monishdeb can you review this? I think it should go in 5.3 as related work is in 5.3 but that means it should be soon |
@monishdeb ping |
Ok will review it by today or tomorrow. |
thx |
(CiviCRM Review Template WORD-1.1)
|
thanks @monishdeb |
Overview
Follow up fixes to #12193 for obscure dedupe scenarios
Before
civicrm/contact/dedupefind?reset=1&action=update&rgid=4&criteria=%7B%22contact%22%3A%7B%22id%22%3A%7B%22IN%22%3A%5B%22506191%22%5D%7D%7D%7D&limit=1
After
limit parameter re-instated (note the results are cached so you need to truncate civicrm_prevnext_cache when changing the value). Also note that 'limit' is the number of contacts to try to find matches for - so if a group has 10000 contacts in it the dedupe finder will only seek to find matches for the first contact if limit=1. (for test purposes I'm using a group of contacts who all have the same email address
toggle works again
Technical Details
I've put this against the rc because it is an addendum to a patch already in the rc. The scenarios described are pretty obscure!
Comments
@monishdeb are you able to look.