Page MenuHomePhabricator

Prevent target smart data from being lost on legacy merge screen
Open, Needs TriagePublic

Event Timeline

Eileenmcnaughton renamed this task from Restore target smart data to Prevent target smart data from being lost on legacy merge screen.Oct 14 2019, 10:42 PM
Eileenmcnaughton added a comment.EditedOct 14 2019, 11:15 PM

I spent a while trying to replicate this & came to the conclusion that

  1. most instances seemed to be cases where the lower contact id had been merged into the higher contact id (indicating the contacts had been 'flipped')
  2. the checkboxes to carry across the data were not checked.

Generally I found this hard to replicate until I realised that I was sticking with merging the newest into the oldest (as the script and the deduper do) but the cases where data was lost generally were the reverse.

In general a lot more care needs to be taken on the old screen because it does not apply rules - for example the Deduper will handle the addresses according to the same rule used for the script, whereas the legacy screen requires the user to specify the way to handle the address - leading to the need for flipping & the ease of missing checking some boxes. While this can be prevented by users being 'careful' it seems better we think about UI ways to prevent mistakes or simply ignore user input & handle by script.

Change 543747 had a related patch set uploaded (by Eileen; owner: Eileen):
[wikimedia/fundraising/crm@master] Add setting to move dedupe over to extension

https://gerrit.wikimedia.org/r/543747

Change 543747 abandoned by Eileen:
Move logic for handling Yes-No fields during dedupe from wmf_civicrm to deduper.

https://gerrit.wikimedia.org/r/543747

Change 545695 had a related patch set uploaded (by Eileen; owner: Eileen):
[wikimedia/fundraising/crm/civicrm@master] Fix dedupe screen to always set the checkbox to 'ticked' where data would otherwise be lost

https://gerrit.wikimedia.org/r/545695

After a bit of code diving I came up with a fix that makes it checked by default if the contact about to be removed has data in any (non location) field and the contact being kept does not - this would apply to prospecting fields & also to fields like middle name or other custom fields

Some screenshots here
https://github.com/civicrm/civicrm-core/pull/15595

@mbeat @NNichols @RLewis does this seem OK as a fix. It is a small change in behaviour for users of the dedupe screen but one that should make it harder to accidentally loose data (at the expense of it being easier to accidentally keep rubbish data

I think that sounds like a great solution!

Change 545695 merged by jenkins-bot:
[wikimedia/fundraising/crm/civicrm@master] Fix dedupe screen to always set the checkbox to 'ticked' where data would otherwise be lost

https://gerrit.wikimedia.org/r/545695

@MBeat33 sorry I mis-pinged above - this change has been made

Sounds good to me.