{"action":"created","issue":{"url":"https://api.github.com/repos/civicrm/civicrm-core/issues/8360","repository_url":"https://api.github.com/repos/civicrm/civicrm-core","labels_url":"https://api.github.com/repos/civicrm/civicrm-core/issues/8360/labels{/name}","comments_url":"https://api.github.com/repos/civicrm/civicrm-core/issues/8360/comments","events_url":"https://api.github.com/repos/civicrm/civicrm-core/issues/8360/events","html_url":"https://github.com/civicrm/civicrm-core/pull/8360","id":154632561,"number":8360,"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},"labels":[{"url":"https://api.github.com/repos/civicrm/civicrm-core/labels/master","name":"master","color":"0052cc"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":10,"created_at":"2016-05-13T04:38:41Z","updated_at":"2016-05-17T09:16:16Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/civicrm/civicrm-core/pulls/8360","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"},"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)"},"comment":{"url":"https://api.github.com/repos/civicrm/civicrm-core/issues/comments/219663092","html_url":"https://github.com/civicrm/civicrm-core/pull/8360#issuecomment-219663092","issue_url":"https://api.github.com/repos/civicrm/civicrm-core/issues/8360","id":219663092,"user":{"login":"JKingsnorth","id":5212601,"avatar_url":"https://avatars.githubusercontent.com/u/5212601?v=3","gravatar_id":"","url":"https://api.github.com/users/JKingsnorth","html_url":"https://github.com/JKingsnorth","followers_url":"https://api.github.com/users/JKingsnorth/followers","following_url":"https://api.github.com/users/JKingsnorth/following{/other_user}","gists_url":"https://api.github.com/users/JKingsnorth/gists{/gist_id}","starred_url":"https://api.github.com/users/JKingsnorth/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/JKingsnorth/subscriptions","organizations_url":"https://api.github.com/users/JKingsnorth/orgs","repos_url":"https://api.github.com/users/JKingsnorth/repos","events_url":"https://api.github.com/users/JKingsnorth/events{/privacy}","received_events_url":"https://api.github.com/users/JKingsnorth/received_events","type":"User","site_admin":false},"created_at":"2016-05-17T09:16:16Z","updated_at":"2016-05-17T09:16:16Z","body":"Looks good @eileenmcnaughton . I didn't think Jenkins ran those tests - but he does and they pass. Hooray. Looks like there are still some tests to add, but the functionality still works after these changes, so I would say good to merge.\r\n\r\n@deepak-srivastava will you have another chance to check this before it's merged in?"}}