Add a unique event ID so we can match pre/post Insert/Update

[mattwire]

Overview

There are two ways to listen for changes to entities via pre/post. Old method via the hook_civicrm_pre/post hooks or new/standardised symfony events civi.dao.preInsert/civi.dao.postInsert/civi.dao.preUpdate/civi.dao.postUpdate

There is no way to link a preInsert/Update event with a postInsert/Update event for the same entity/change. Normally this doesn't matter but for a "busy" site or where there are multiple things happening based on a change to an entity this is problematic.

This came up specifically on a site with:

1. CiviRules configured with a number of Activity "Old value is X / New value is Y" where you need to store the original data (retrieve it from the DB during the pre) and compare with the new data (compare during the post).
2. Another extension using Activity post hook to calculate some "scores" based on submitted custom values and save back to the activity in another custom field.

This resulted in inconsistent behaviour which we tracked down to timing issues. Sometimes the pre-hook fired for both 1 and 2 BEFORE the post hook had fired for the relevant change. Resulting in the "original data" being overwritten by the second pre-hook and not matching the change that was being checked in the post hook.

I've come across similar "challenges" before and being able to match up the pre/post for the same change would make things much easier. With the work that @colemanw has done for API4/standardisation of the DAO this is now possible in just 5 lines of code.

Before

Not possible to link a pre/post Insert/Update event for the same change.

After

Can use the eventID to link a pre/post Insert/Update event for the same change.

Technical Details

I used uniqID() but it really doesn't matter too much as long as the ID is unique in the context of the events firing at the time.
I added eventID as a property to the event separate to the entity params.

Comments

@colemanw @seamuslee001 @eileenmcnaughton What do you think? This seems like a nice simple solution to a problem that's come up for me multiple times and most likely has for others too!

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

[christianwach]

Excellent idea @mattwire

[eileenmcnaughton]

I don't see a problem with this - but I'd like @totten to confirm that there is not some alternative he would like to propose

[seamuslee001]

I'm a +1 on this for a moment I got thrown by EventId thinking === civicrm_event.id (when I saw this on chat) but then I read the description + patch

[yashodha]

@mattwire looks good.

[eileenmcnaughton]

Various supportive comments - merging