Alter dedupe code to call api rather than bao->save()

[eileenmcnaughton]

Overview

Alter dedupe code to call api rather than bao->save()

This achieves 2 things

1. hooks are no longer bypassed
2. it leverages core handling for is_primary rather than this somewhat unreliable attempt

Before

$bao->save()

After

api call.

Technical Details

I feel pretty sure we've had requests to not bypass hooks in the past but my current reason is that I can't figure out a way to handle a particular situation in an extension - this is where the contacts have the same email but one is on hold and I want to update the email to be on hold but not wind up with no primary emails (which is what is happening) and not make a change before the dedupe actually runs.

If we call the BAO::create (via the api) then we get the benefit of core is_primary handling

Comments

@JKingsnorth @pfigel are either of you up for review on some more deduper work

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[JKingsnorth]

Hi @eileenmcnaughton - I can certainly run it at least =]

[eileenmcnaughton]

@JKingsnorth I've updated this code to have the actual change in it now

[JKingsnorth]

@eileenmcnaughton this is on my list for tomorrow. We actually have some custom code that forces location entity hooks to trigger during the merge process, so I'm going to do a thorough test of before and after this, including how the hooks behave =] Sorry for the delay.

[eileenmcnaughton]

Thanks @JKingsnorth

[JKingsnorth]

@eileenmcnaughton

I tested this by merging two contacts, with an address, email, phone and website each. The merge completed successfully, and overwrote the location entitles, as desired.

Before the patch, these hooks were called on merge:

Hooked _pre: update / Individual / 4689779 / Params follow
Hooked _post: update / Individual / 4689779 / objectRef follows
Hooked _pre: edit / Individual / 4689778 / Params follow
Hooked _post: edit / Individual / 4689778 / objectRef follows
Hooked _post: merge / Contact / 4689778 / objectRef follows
Hooked _pre: create / Activity /  / Params follow
Hooked _post: create / Activity / 3128578 / objectRef follows
Hooked _pre: create / Activity /  / Params follow
Hooked _post: create / Activity / 3128579 / objectRef follows

After the patch:

Hooked _pre: edit / Address / 7286822 / Params follow
Hooked _custom: edit / 222 / 7286822 / Params follow
Hooked _post: edit / Address / 7286822 / objectRef follows
Hooked _pre: delete / Address / 7286821 / Params follow
Hooked _post: delete / Address / 7286821 / objectRef follows
Hooked _pre: edit / Email / 4873407 / Params follow
Hooked _post: edit / Email / 4873407 / objectRef follows
Hooked _pre: delete / Email / 4873406 / Params follow
Hooked _post: delete / Email / 4873406 / objectRef follows
Hooked _pre: edit / Phone / 3837577 / Params follow
Hooked _post: edit / Phone / 3837577 / objectRef follows
Hooked _pre: delete / Phone / 3837576 / Params follow
Hooked _post: delete / Phone / 3837576 / objectRef follows
Hooked _pre: edit / Website / 19122 / Params follow
Hooked _post: edit / Website / 19122 / objectRef follows
Hooked _pre: delete / Website / 19121 / Params follow
Hooked _post: delete / Website / 19121 / objectRef follows
Hooked _pre: update / Individual / 4689783 / Params follow
Hooked _post: update / Individual / 4689783 / objectRef follows
Hooked _pre: edit / Individual / 4689782 / Params follow
Hooked _custom: edit / 142 / 4689782 / Params follow
Hooked _post: edit / Individual / 4689782 / objectRef follows
Hooked _post: merge / Contact / 4689782 / objectRef follows
Hooked _pre: create / Activity /  / Params follow
Hooked _post: create / Activity / 3128582 / objectRef follows
Hooked _pre: create / Activity /  / Params follow
Hooked _post: create / Activity / 3128583 / objectRef follows

The address I tested also had a custom field attached, which came across OK too.

So this all looks good to me!

---

2 problems I found which exist in core without this patch...

Notice: Undefined index: is_primary in CRM_Dedupe_Merger::addLocationFieldInfo() (line 2412 of .../civicrm/CRM/Dedupe/Merger.php).

and more seriously...

During merge I chose NOT to take a website across, but it came across to the contact anyway. Shall I raise a gitlab for that @eileenmcnaughton ?

This only happened with the 'website' entity.

[eileenmcnaughton]

@JKingsnorth yes - please log in gitlab

@seamuslee001 can you merge based on ^^

[seamuslee001]

Merging based on @JKingsnorth 's review