{"action":"created","comment":{"url":"https://api.github.com/repos/civicrm/civicrm-core/pulls/comments/63430574","id":63430574,"diff_hunk":"@@ -586,36 +586,23 @@ public static function retrieveFields($main, $other) {\n    *                              A 'safe' value skips the merge if there are any un-resolved conflicts.\n    *                              Does a force merge otherwise.\n    * @param bool $autoFlip to let api decide which contact to retain and which to delete.\n-   *   Wether to let api decide which contact to retain and which to delete.\n+   *   Whether to let api decide which contact to retain and which to delete.\n    * @param int $batchLimit number of merges to carry out in one batch.\n    * @param int $isSelected if records with is_selected column needs to be processed.\n    *\n    * @return array|bool\n    */\n   public static function batchMerge($rgid, $gid = NULL, $mode = 'safe', $autoFlip = TRUE, $batchLimit = 1, $isSelected = 2) {\n-    $cacheKeyString = CRM_Dedupe_Merger::getMergeCacheKeyString($rgid, $gid);\n-    $join = CRM_Dedupe_Merger::getJoinOnDedupeTable();\n-\n-    $where = \"de.id IS NULL\";\n-    if ($isSelected === 0 || $isSelected === 1) {\n-      $where .= \" AND pn.is_selected = {$isSelected}\";\n-    }// else consider all dupe pairs\n-    $where .= \" LIMIT {$batchLimit}\";\n-\n     $redirectForPerformance = ($batchLimit > 1) ? TRUE : FALSE;\n-\n-    $dupePairs = CRM_Core_BAO_PrevNextCache::retrieve($cacheKeyString, $join, $where);\n-    if (empty($dupePairs) && !$redirectForPerformance && $isSelected == 2) {\n-      // If we haven't found any dupes, probably cache is empty.\n-      // Try filling cache and give another try.\n-      CRM_Core_BAO_PrevNextCache::refillCache($rgid, $gid, $cacheKeyString);\n-      $dupePairs = CRM_Core_BAO_PrevNextCache::retrieve($cacheKeyString, $join, $where);\n-    }\n+    $reloadCacheIfEmpty = (!$redirectForPerformance && $isSelected == 2);\n+    $dupePairs = self::getDuplicatePairs($rgid, $gid, $reloadCacheIfEmpty, $batchLimit, $isSelected, '', FALSE);","path":"CRM/Dedupe/Merger.php","position":31,"original_position":31,"commit_id":"704ae12584e5f866a9d36af282d5b27041c74910","original_commit_id":"704ae12584e5f866a9d36af282d5b27041c74910","user":{"login":"eileenmcnaughton","id":336308,"avatar_url":"https://avatars.githubusercontent.com/u/336308?v=3","gravatar_id":"","url":"https://api.github.com/users/eileenmcnaughton","html_url":"https://github.com/eileenmcnaughton","followers_url":"https://api.github.com/users/eileenmcnaughton/followers","following_url":"https://api.github.com/users/eileenmcnaughton/following{/other_user}","gists_url":"https://api.github.com/users/eileenmcnaughton/gists{/gist_id}","starred_url":"https://api.github.com/users/eileenmcnaughton/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/eileenmcnaughton/subscriptions","organizations_url":"https://api.github.com/users/eileenmcnaughton/orgs","repos_url":"https://api.github.com/users/eileenmcnaughton/repos","events_url":"https://api.github.com/users/eileenmcnaughton/events{/privacy}","received_events_url":"https://api.github.com/users/eileenmcnaughton/received_events","type":"User","site_admin":false},"body":"OK - now I understand the 'flip-flop' nature of the problem. I don't personally like the idea of the getDuplicates function deciding based on $mode - because getDuplicates is logically potentially for use cases broader than just merge & it feels like putting higher level logic into a lower level function - but if I am going to pass in includeConflicts that needs to be accurate to the mode! \r\n\r\nAnyway I'll dig on that now","created_at":"2016-05-16T21:37:52Z","updated_at":"2016-05-16T21:37:52Z","html_url":"https://github.com/civicrm/civicrm-core/pull/8360#discussion_r63430574","pull_request_url":"https://api.github.com/repos/civicrm/civicrm-core/pulls/8360","_links":{"self":{"href":"https://api.github.com/repos/civicrm/civicrm-core/pulls/comments/63430574"},"html":{"href":"https://github.com/civicrm/civicrm-core/pull/8360#discussion_r63430574"},"pull_request":{"href":"https://api.github.com/repos/civicrm/civicrm-core/pulls/8360"}}},"pull_request":{"url":"https://api.github.com/repos/civicrm/civicrm-core/pulls/8360","id":69942426,"html_url":"https://github.com/civicrm/civicrm-core/pull/8360","diff_url":"https://github.com/civicrm/civicrm-core/pull/8360.diff","patch_url":"https://github.com/civicrm/civicrm-core/pull/8360.patch","issue_url":"https://api.github.com/repos/civicrm/civicrm-core/issues/8360","number":8360,"state":"open","locked":false,"title":"CRM-18517 fix merge code loop","user":{"login":"eileenmcnaughton","id":336308,"avatar_url":"https://avatars.githubusercontent.com/u/336308?v=3","gravatar_id":"","url":"https://api.github.com/users/eileenmcnaughton","html_url":"https://github.com/eileenmcnaughton","followers_url":"https://api.github.com/users/eileenmcnaughton/followers","following_url":"https://api.github.com/users/eileenmcnaughton/following{/other_user}","gists_url":"https://api.github.com/users/eileenmcnaughton/gists{/gist_id}","starred_url":"https://api.github.com/users/eileenmcnaughton/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/eileenmcnaughton/subscriptions","organizations_url":"https://api.github.com/users/eileenmcnaughton/orgs","repos_url":"https://api.github.com/users/eileenmcnaughton/repos","events_url":"https://api.github.com/users/eileenmcnaughton/events{/privacy}","received_events_url":"https://api.github.com/users/eileenmcnaughton/received_events","type":"User","site_admin":false},"body":"This code fixes the code loop in CRM-18517. It was built on some other refactorings and other attempts to add tests (which failed due to the code loop). Despite all the code the only functional change is the bit that removes the code loop.\r\n\r\nAll of this is basically preliminary tidy up / getting testing to an OK state in order to address \r\n\r\n- CRM-18546 Duplicate email addresses created when merging in 4.7\r\n- CRM-18539 Permit criteria for batch merging (other than group) and a limit\r\n- CRM-18442 (unncessary) Slow query for dedupes\r\n\r\nWhich is pretty much a minimum set of fixes for us to do batch mode deduping, and which are all blocked on my ability to write proper unit tests.\r\n\r\nNote as to what I have observed in 4.7 but not yet looked at (and see as non-regressive)\r\n1) persistence on the merge screen  - e.g if you choose to show address data & then merge & then go back it has lost that.\r\n2) the ajax function really needs a test of it's own & cleaner parameter retrieval\r\n3) the search functions on the ajax screen don't respect the wildcard setting - which would potentially result in slow queries\r\n\r\nOur last requirement to permit batch merging from the UI would be to impose a limit - so that if someone accidentally choses a group with 500,000 people it in the server won't crash. You can't say 'retrieve first 1000' but you could iterate in batches of say 500 contacts & if you get to 1000 matches maybe return & say 'you want more? Do ya punk?'\n\n---\n\n * [CRM-18517: code loop when trying to dedupe through the batch merge job if conflicts exist](https://issues.civicrm.org/jira/browse/CRM-18517)\n * [CRM-18546: Duplicate email addresses created when merging in 4.7](https://issues.civicrm.org/jira/browse/CRM-18546)\n * [CRM-18539: Permit criteria for batch merging \\(other than group\\) and a limit](https://issues.civicrm.org/jira/browse/CRM-18539)\n * [CRM-18442: \\(unncessary\\) Slow query for dedupes](https://issues.civicrm.org/jira/browse/CRM-18442)","created_at":"2016-05-13T04:38:41Z","updated_at":"2016-05-16T21:37:52Z","closed_at":null,"merged_at":null,"merge_commit_sha":"54d94eb08318280ebe2d368dfa22ca58407353cb","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/civicrm/civicrm-core/pulls/8360/commits","review_comments_url":"https://api.github.com/repos/civicrm/civicrm-core/pulls/8360/comments","review_comment_url":"https://api.github.com/repos/civicrm/civicrm-core/pulls/comments{/number}","comments_url":"https://api.github.com/repos/civicrm/civicrm-core/issues/8360/comments","statuses_url":"https://api.github.com/repos/civicrm/civicrm-core/statuses/704ae12584e5f866a9d36af282d5b27041c74910","head":{"label":"eileenmcnaughton:prev_next","ref":"prev_next","sha":"704ae12584e5f866a9d36af282d5b27041c74910","user":{"login":"eileenmcnaughton","id":336308,"avatar_url":"https://avatars.githubusercontent.com/u/336308?v=3","gravatar_id":"","url":"https://api.github.com/users/eileenmcnaughton","html_url":"https://github.com/eileenmcnaughton","followers_url":"https://api.github.com/users/eileenmcnaughton/followers","following_url":"https://api.github.com/users/eileenmcnaughton/following{/other_user}","gists_url":"https://api.github.com/users/eileenmcnaughton/gists{/gist_id}","starred_url":"https://api.github.com/users/eileenmcnaughton/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/eileenmcnaughton/subscriptions","organizations_url":"https://api.github.com/users/eileenmcnaughton/orgs","repos_url":"https://api.github.com/users/eileenmcnaughton/repos","events_url":"https://api.github.com/users/eileenmcnaughton/events{/privacy}","received_events_url":"https://api.github.com/users/eileenmcnaughton/received_events","type":"User","site_admin":false},"repo":{"id":8567814,"name":"civicrm-core","full_name":"eileenmcnaughton/civicrm-core","owner":{"login":"eileenmcnaughton","id":336308,"avatar_url":"https://avatars.githubusercontent.com/u/336308?v=3","gravatar_id":"","url":"https://api.github.com/users/eileenmcnaughton","html_url":"https://github.com/eileenmcnaughton","followers_url":"https://api.github.com/users/eileenmcnaughton/followers","following_url":"https://api.github.com/users/eileenmcnaughton/following{/other_user}","gists_url":"https://api.github.com/users/eileenmcnaughton/gists{/gist_id}","starred_url":"https://api.github.com/users/eileenmcnaughton/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/eileenmcnaughton/subscriptions","organizations_url":"https://api.github.com/users/eileenmcnaughton/orgs","repos_url":"https://api.github.com/users/eileenmcnaughton/repos","events_url":"https://api.github.com/users/eileenmcnaughton/events{/privacy}","received_events_url":"https://api.github.com/users/eileenmcnaughton/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/eileenmcnaughton/civicrm-core","description":"CiviCRM (Core Application and Framework)","fork":true,"url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core","forks_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/forks","keys_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/keys{/key_id}","collaborators_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/teams","hooks_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/hooks","issue_events_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/issues/events{/number}","events_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/events","assignees_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/assignees{/user}","branches_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/branches{/branch}","tags_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/tags","blobs_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/git/refs{/sha}","trees_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/git/trees{/sha}","statuses_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/statuses/{sha}","languages_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/languages","stargazers_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/stargazers","contributors_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/contributors","subscribers_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/subscribers","subscription_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/subscription","commits_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/commits{/sha}","git_commits_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/git/commits{/sha}","comments_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/comments{/number}","issue_comment_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/issues/comments{/number}","contents_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/contents/{+path}","compare_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/compare/{base}...{head}","merges_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/merges","archive_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/downloads","issues_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/issues{/number}","pulls_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/pulls{/number}","milestones_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/milestones{/number}","notifications_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/labels{/name}","releases_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/releases{/id}","deployments_url":"https://api.github.com/repos/eileenmcnaughton/civicrm-core/deployments","created_at":"2013-03-04T23:49:52Z","updated_at":"2015-03-23T03:03:28Z","pushed_at":"2016-05-16T21:28:05Z","git_url":"git://github.com/eileenmcnaughton/civicrm-core.git","ssh_url":"git@github.com:eileenmcnaughton/civicrm-core.git","clone_url":"https://github.com/eileenmcnaughton/civicrm-core.git","svn_url":"https://github.com/eileenmcnaughton/civicrm-core","homepage":null,"size":152332,"stargazers_count":1,"watchers_count":1,"language":"PHP","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":1,"mirror_url":null,"open_issues_count":0,"forks":1,"open_issues":0,"watchers":1,"default_branch":"code_ideas"}},"base":{"label":"civicrm:master","ref":"master","sha":"af6b2380fbac6645952be28839153f287a681358","user":{"login":"civicrm","id":136373,"avatar_url":"https://avatars.githubusercontent.com/u/136373?v=3","gravatar_id":"","url":"https://api.github.com/users/civicrm","html_url":"https://github.com/civicrm","followers_url":"https://api.github.com/users/civicrm/followers","following_url":"https://api.github.com/users/civicrm/following{/other_user}","gists_url":"https://api.github.com/users/civicrm/gists{/gist_id}","starred_url":"https://api.github.com/users/civicrm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/civicrm/subscriptions","organizations_url":"https://api.github.com/users/civicrm/orgs","repos_url":"https://api.github.com/users/civicrm/repos","events_url":"https://api.github.com/users/civicrm/events{/privacy}","received_events_url":"https://api.github.com/users/civicrm/received_events","type":"Organization","site_admin":false},"repo":{"id":8495499,"name":"civicrm-core","full_name":"civicrm/civicrm-core","owner":{"login":"civicrm","id":136373,"avatar_url":"https://avatars.githubusercontent.com/u/136373?v=3","gravatar_id":"","url":"https://api.github.com/users/civicrm","html_url":"https://github.com/civicrm","followers_url":"https://api.github.com/users/civicrm/followers","following_url":"https://api.github.com/users/civicrm/following{/other_user}","gists_url":"https://api.github.com/users/civicrm/gists{/gist_id}","starred_url":"https://api.github.com/users/civicrm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/civicrm/subscriptions","organizations_url":"https://api.github.com/users/civicrm/orgs","repos_url":"https://api.github.com/users/civicrm/repos","events_url":"https://api.github.com/users/civicrm/events{/privacy}","received_events_url":"https://api.github.com/users/civicrm/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/civicrm/civicrm-core","description":"CiviCRM (Core Application and Framework)","fork":false,"url":"https://api.github.com/repos/civicrm/civicrm-core","forks_url":"https://api.github.com/repos/civicrm/civicrm-core/forks","keys_url":"https://api.github.com/repos/civicrm/civicrm-core/keys{/key_id}","collaborators_url":"https://api.github.com/repos/civicrm/civicrm-core/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/civicrm/civicrm-core/teams","hooks_url":"https://api.github.com/repos/civicrm/civicrm-core/hooks","issue_events_url":"https://api.github.com/repos/civicrm/civicrm-core/issues/events{/number}","events_url":"https://api.github.com/repos/civicrm/civicrm-core/events","assignees_url":"https://api.github.com/repos/civicrm/civicrm-core/assignees{/user}","branches_url":"https://api.github.com/repos/civicrm/civicrm-core/branches{/branch}","tags_url":"https://api.github.com/repos/civicrm/civicrm-core/tags","blobs_url":"https://api.github.com/repos/civicrm/civicrm-core/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/civicrm/civicrm-core/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/civicrm/civicrm-core/git/refs{/sha}","trees_url":"https://api.github.com/repos/civicrm/civicrm-core/git/trees{/sha}","statuses_url":"https://api.github.com/repos/civicrm/civicrm-core/statuses/{sha}","languages_url":"https://api.github.com/repos/civicrm/civicrm-core/languages","stargazers_url":"https://api.github.com/repos/civicrm/civicrm-core/stargazers","contributors_url":"https://api.github.com/repos/civicrm/civicrm-core/contributors","subscribers_url":"https://api.github.com/repos/civicrm/civicrm-core/subscribers","subscription_url":"https://api.github.com/repos/civicrm/civicrm-core/subscription","commits_url":"https://api.github.com/repos/civicrm/civicrm-core/commits{/sha}","git_commits_url":"https://api.github.com/repos/civicrm/civicrm-core/git/commits{/sha}","comments_url":"https://api.github.com/repos/civicrm/civicrm-core/comments{/number}","issue_comment_url":"https://api.github.com/repos/civicrm/civicrm-core/issues/comments{/number}","contents_url":"https://api.github.com/repos/civicrm/civicrm-core/contents/{+path}","compare_url":"https://api.github.com/repos/civicrm/civicrm-core/compare/{base}...{head}","merges_url":"https://api.github.com/repos/civicrm/civicrm-core/merges","archive_url":"https://api.github.com/repos/civicrm/civicrm-core/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/civicrm/civicrm-core/downloads","issues_url":"https://api.github.com/repos/civicrm/civicrm-core/issues{/number}","pulls_url":"https://api.github.com/repos/civicrm/civicrm-core/pulls{/number}","milestones_url":"https://api.github.com/repos/civicrm/civicrm-core/milestones{/number}","notifications_url":"https://api.github.com/repos/civicrm/civicrm-core/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/civicrm/civicrm-core/labels{/name}","releases_url":"https://api.github.com/repos/civicrm/civicrm-core/releases{/id}","deployments_url":"https://api.github.com/repos/civicrm/civicrm-core/deployments","created_at":"2013-03-01T06:11:56Z","updated_at":"2016-05-04T03:27:08Z","pushed_at":"2016-05-16T21:11:47Z","git_url":"git://github.com/civicrm/civicrm-core.git","ssh_url":"git@github.com:civicrm/civicrm-core.git","clone_url":"https://github.com/civicrm/civicrm-core.git","svn_url":"https://github.com/civicrm/civicrm-core","homepage":null,"size":165364,"stargazers_count":150,"watchers_count":150,"language":"PHP","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":415,"mirror_url":null,"open_issues_count":153,"forks":415,"open_issues":153,"watchers":150,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/civicrm/civicrm-core/pulls/8360"},"html":{"href":"https://github.com/civicrm/civicrm-core/pull/8360"},"issue":{"href":"https://api.github.com/repos/civicrm/civicrm-core/issues/8360"},"comments":{"href":"https://api.github.com/repos/civicrm/civicrm-core/issues/8360/comments"},"review_comments":{"href":"https://api.github.com/repos/civicrm/civicrm-core/pulls/8360/comments"},"review_comment":{"href":"https://api.github.com/repos/civicrm/civicrm-core/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/civicrm/civicrm-core/pulls/8360/commits"},"statuses":{"href":"https://api.github.com/repos/civicrm/civicrm-core/statuses/704ae12584e5f866a9d36af282d5b27041c74910"}}}}