(dev/core#3045) ManagedEntities - Fix crash during upgrade

[colemanw]

Overview

This prevents a potential crash when trying to update a column before it has been added by the upgrader

Fixes dev/core#3045

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

[totten]

Part A

I've been trying to replicate the report... I installed 5.33 and tried doing all these things to the initial 5.33 DB:

• Add new extension and run civix generate:report (to create a managed entity)
• Create smart groups via traditional search. One just filtering on name; one filtering on event start date.
• Install search. Created a saved search searchkit UI.
• Install deduper, extendedreport. (They show up in some of the reports.)
• Hack civicrm_managed and deduper so that one of the installed extensions appears to take ownership of my manually-created saved-search.

Then I ran DB upgrades -- with both the web-based upgrader and cv upgrade:db (cv v0.3.15).

Part B

Something in the backtrace and patch is confusing to me -- it appears to be firing hook_civicrm_post during incremental schema-updates. The dispatch policy is to shush hooks during incremental schema-updates. (Precisely because external hook listeners generally aren't attuned -- and generally cannot be attuned -- to all the wonky interim states of an upgrading DB...)

While trying to r-run above, I also edited CRM_Core_BAO_Managed::on_hook_civicrm_post to log whenever it's called, and it wasn't called. This could just be that I haven't understood the data/configuration needed to reproduce (see Part A) -- or it could be that it's not fired normally, and that there's some special circumstance which causes it fire unexpectedly.

I struggle to see how the backtrace would arise. - or how the patch here could be operative. Maybe the system has additional/inter-related patches? Maybe it's an old version of cv that doesn't respect the dispatch-policy? Maybe it's a different upgrader? (I don't know -- I'm just throwing spaghetti at the wall.)

Part C

On a concrete level, I can't reproduce; on an abstract level, I don't understand. So... I don't really have enough to go on for merging...

Of course, if someone else can test/reproduce, then that's cool...

[colemanw]

@totten your first step may be the problem. civix generate:report will make a managed record but not an APIv4 ManagedEntity type. Those include ContactType, CustomField, CustomGroup, Dashboard, Group, MembershipStatus, MembershipType, Navigation, OptionGroup, OptionValue, PaymentProcessorType, RelationshipType, SavedSearch & SearchDisplay.

[totten]

@colemanw Yes, that's a valid point about the first attempt. But it doesn't explain the third attempt (since 5.33's search registered mgd's that use OptionGroup and OptionValue), and it doesn't explain the fifth attempt (which had mgd for a SavedSearch record).

(Note: My bulleted list may have been a little confusing -- it wasn't really "first step; second step" in a procedure. It was more like: "I tried one approach, and it didn't work; then I tried a second approach, which also didn't work". Basically, after every intervention, I re-ran the upgrade - and the upgrade ran fine - so I'd move on to try another approach.)

[seamuslee001]

I'm going to merge based on the review on #22643 from 2 separate community members saying this fixed problems for them