(dev/core#3093) CiviGrant - Normalize auto-activation during upgrade

[totten]

Overview

During the upgrade process the CiviGrant extension may be installed. This ensures its dependencies are also enabled. It is an alternative to #22861

Fixes https://lab.civicrm.org/dev/core/-/issues/3093

ping @colemanw @demeritcowboy @seamuslee001

Before

The FiveFortySeven upgrade partially activates the civigrant extension. It does not activate dependencies like afform and search_kit.

The step which enables civigrant runs during middle of core schema-upgrades; consequently, it plays by the rules of "assume many high-level services are unavailable".

After

The FiveFortySeven upgrade fully activates the CiviGrant extension along with its dependencies (whatever they may be at time of execution; eg afform and search_kit).

The step which enables civigrant runs after the core schema-upgrades; consequently, it can use more conventional services to activate extensions (and their dependencies).

Technical Details

When upgrading, the CRM/Upgrade system builds a queue of upgrade-tasks. The key thing here is to schedule any new extension activation at the end (after core schema-upgrades). This is accomplished by setting the weight explicitly.

There are a few implications of activating at the end. I don't think they are problematic - but are worth stating out loud:

• It will always use the current civigrant installer at the time of upgrade. If the installer changes, future-upgrade-users will run the then-current installer.
• All those managed entities are being setup based on the civigrant *.mgd.php files. I removed the long list of managed bits from migrateCiviGrant() -- based on the (unverified) assumption that they are equivalent to the normal definitions. (If there is some reason why they need to be different from the normal *.mgd.php, then we should examine that...)
• If there are future schema revisions needed for civigrant, they should probably be placed in ext/civigrant rather than doing core schema-upgrades. There could be some edge-case quirks in either direction, but I think it would be decidedly confusing to have civigrant schema changes executing before civigrant is installed.

We're basically handing-off responsibility for CiviGrant schema from CRM/Upgrade to ext/civigrant.

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

[colemanw]

@totten the grant install code you deleted may look pointless at first glance, but it serves a purpose, namely converting records which already exist to managed entities.

Remember that components do not behave quite like extensions. They can be disabled but never uninstalled. And all their tables and config data are present whether you ever use the component or not. So the job here is to match up all the existing stuff which got installed for the CiviGrant component and slip it into the civicrm_managed table to fool the system into thinking it was created as a managed entity for the CiviGrant extension.

Unfortunately, what you did here is going to cause the records to be double-inserted, and will likely crash the upgrader.

[demeritcowboy]

I'm not seeing duplicates but a bigger problem is all my existing grants are gone! It's not immediately obvious why, but I'm not imagining it because I happened to have logging turned on and I can see them in the log_civicrm_grant table.

And I do still get the Type Error from before when I visit a contact record or any search-kit'ified page. And it doesn't go away after clearing cache.

What might be related to the managed items is it seems to now create a new Reports submenu for Grant Reports but with nothing in it - the grant reports were and still are under Contact Reports. I don't remember it doing that before.

Less important note: After upgrade you get a system check warning about missing log tables (it's obviously about civicrm_search_display). Easily fixable though.

I also see another separate thing - will make a ticket.

[colemanw]

I feel like this line might fix the problems people have reported over the years of the upgrader not completely flushing caches, because this one runs with a full bootstrap. Maybe we should do this unconditionally at the end of every upgrade.

[totten]

We already do! (Per doCoreFinish() nee doFinish()) This is an additional call just to deal with the extension. (You may have noticed this after posting.)

FWIW, there is a subtle difference, and I'm on the fence about it:

• enableExtension() passes $triggerRebuild=TRUE
• doFinish() passes $triggerRebuild=FALSE and compensates with fixSchemaDifferences()

It's also arguably a bit redundant to have two calls. One call (scheduled at, say, weight=3000 instead of weight=1000) might do the job. But I think the pre-conditions/post-conditions are a bit clearer with the two calls - so I'd expect it to be less brittle in the face of future changes.

[totten]

...converting records which already exist to managed entities.

OK, makes perfect sense. Closing in favor of the other PR. Hopefully the managed-entity part will address a few of the issues demerit saw.

[colemanw]

...converting records which already exist to managed entities.

OK, makes perfect sense. Closing in favor of the other PR. Hopefully the managed-entity part will address a few of the issues demerit saw.

Tangentially, this PR should help with converting existing records to managed entities in the future: #22883