Fix Dedupe entity_tag mangling bug

[eileenmcnaughton]

Overview

Fixes a bug whereby the merge script alters non-contact entity tags, changing the entity they are related to

Before

Start with a contact & an activity with the same id

Assign a contact entity tag to the contact and an activity entity tag to the activity.

Merge the contact with another contact, note that the entity_id is updated on the activity entity_tag too

After

Only the contact entity tag is updated.

Technical Details

I identified this bug in testing removing the UPDATE IGNORE + DELETE in #17072.

entity_tag is hack-added to the array of tables that interact with contact - resulting in it passing through the UPDATE without the WHERE entity_table = ...

The function with the hack-add is called from 2 places. We know it is unwanted in 1 so I moved it to the other place (being unsure if it is useful there this should give no change)

civicrm_entity_tag was missing from the array of tables that are entity_table-linked to contacts. I switched to using discovery rather than hard-coding but I added caching to limit it to once per batch script

Comments

@pfigel

[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).

[eileenmcnaughton]

@pfigel will you be able to look at this?

[pfigel]

@eileenmcnaughton will take a look at this today or tomorrow.

[pfigel]

That's a nasty one - I'm sure we've been bitten by it but never noticed. Thanks @eileenmcnaughton!

• General standards
  • (r-explain) PASS
  • (r-user) PASS: User would expect tags with unrelated entities not to be touched by a merge.
  • (r-doc) PASS
  • (r-run) PASS: Test scenarios:
    • ✅ Tag on activity where entity_id was colliding with the merged contact wasn't updated.
    • ✅ civicrm_entity_file row attached to the merged contact was updated correctly
    • ✅ No broader issues with merge screen/results found
• Developer standards
  • (r-tech) PASS: More standardization. Signature change on CRM_Dedupe_Merger::moveContactBelongings() likely has no ecosystem impact (no universe matches).
  • (r-code) PASS
  • (r-maint) PASS
  • (r-test) PASS

[eileenmcnaughton]

@pfigel we have probably been hit too. I only found it because I saw alarming queries run on our server 'DELETE FROM civicrm_contact WHERE employer_id = x' - which led me to #17072 & then this

[MegaphoneJon]

Is there a Gitlab issue for this?

[eileenmcnaughton]

maybe not - there was a bunch of refactors & this was found adding a test