REF Move event cart to extension

[mattwire]

Overview

Using the new "hidden" core extension functionality to allow us to start moving functionality into extensions.

This creates an eventcart extension and moves a large part of the functionality out of core into the extension.
It does not yet move:

1. "Conference slots". This is a set of functionality that is only enabled when event cart is enabled and requires event cart functionality. However, I decided it was out of scope for this PR because it would require another significant move of files and make this PR unreviewable.
2. A few lines of code that check when event cart is enabled and do specific things. These will not break if eventcart is disabled because the setting would just return false if not defined. Code comments added to flag these.

Before

Event cart part of CiviCRM core.

After

Event cart (mostly) an extension.

Technical Details

Move the eventcart schema, BAO and forms to an extension. Add installer methods in the same way as sequentialcreditnotes.

Comments

An upgrade should be smooth. If event cart was previously enabled you may need to clear caches but that should happen during upgrade anyway.

@bgm @eileenmcnaughton Tests should now be passing (it needed a little bit of work to pull out the schema from core).

To test, enable and disable event cart. Check that event cart functionality such as "Add to Cart", "View Cart" and "Checkout" still works.

[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]

@bgm you might want to track this

@mattwire the current core-extension model allows for the idea that moving stuff to an extension is a process that could take place over several releases - since disabling them is not supported

[mlutfy]

Test result:

+ php [...] GenCode.php schema/Schema.xml '' Drupal

civicrm_domain.version := 5.27.alpha1

Parsing schema description schema/Schema.xml
Extracting database information
Extracting table information
civicrm_event_carts is not a valid foreign key table in civicrm_participant

[mattwire]

@bgm Right, that makes sense because the schema no longer exists in core but in an extension which will load later. I can't see where that FK is used except in event cart (and then I only found one usage). I've just pushed a commit that removes the FK constraint (as we already do that elsewhere in CiviCRM). Let's see if anything else fails.

[eileenmcnaughton]

@mattwire it needs a rebase

[mattwire]

@eileenmcnaughton Rebased

[colemanw]

@mlutfy tests are passing. Can you review?

[colemanw]

@mattwire - I'm really looking forward to being able to remove the system setting 'enable_cart' and simply be able to enable/disable the extension!!!

[mlutfy]

It looks so simple on the surface, but reading the diff.. really impressive work.

About translation: I had commented about E::ts, but we may want to continue using ts on core-extensions and make sure we run string extraction on them?

Ran into an issue enabling it: I did a bit of testing on the demo site:

http://core-17339-2fwtx.test-1.civicrm.org:8001/ (admin / jXm4wSbqdecP)

• Admin > Event component: Enabled the Event Cart
• Events > Dashboard: access an event.

Had a fatal error:

Database Error Code: Table 'core173392fwtxcivi_qk8m3.civicrm_event_carts' doesn't exist, 1146

I noticed that the extension is enabled in the Upgrader by running an SQL function. Is it to avoid a cache-flush from using Extension.enable or equivalent BAO?

Looking at the diff, maybe I missed it, but I couldn't find an equivalent ext/eventcart/sql/install.sql?

[mattwire]

I noticed that the extension is enabled in the Upgrader by running an SQL function. Is it to avoid a cache-flush from using Extension.enable or equivalent BAO?

It's just the way it had been done with sequentialcreditnotes. I guess the difference here is that we actually do need to run some stuff on install, but not on the core upgrade where we split it off. @totten Are you able to help at all with the activation code for this eventcart extension?

[colemanw]

I don't know how far down the road you've gotten, but since core extensions are "always on" you could leave the schema & install/uninstall logic for a future PR.

[mlutfy]

Would it be possible to add a on_change on the enable_cart setting, and if a cart table does not exist, call the "install" function of the extension?

[mattwire]

I've just pushed the civix upgrader as that was missed off too. Basically on install we need a way of calling the extension install/postInstall hooks. I wonder if we could set the schema_version on the extension to something - -1? and the civicrm install knows to trigger install if it finds that value (default is NULL).

[eileenmcnaughton]

the install issue it was pretty tricky last time.

A big part of the reason for hidden core extension was to allow us to not solve everything all at once - e.g we could move most things to the extension but not some tricky thing (in this case the install) and not delay all work on it until we solved the hard parts.

[mlutfy]

In other words: it's a big PR and will go stale quickly, we can merge now and fix the install in a separate PR?

(fine by me, considering the circumstances)

[seamuslee001]

Jenkins re test this please

[eileenmcnaughton]

After discussing in ct catchup this am we have progressed getting some of the stale-ness prone parts of this merged - with the civix generated code merged and this to move the BAO stuff #17743

[eileenmcnaughton]

The big move is merged! I think the way forwards now is small PRs like this one

#17841

so I'll close this & the component pieces can be considered separately

[colemanw]

@mattwire this is stale do you want to keep it open?

[mattwire]

@mattwire this is stale do you want to keep it open?

@colemanw I find it much easier to keep it open as a meta PR while extracting bits out for merge. Have just rebased and pulled out #17891.

[mattwire]

See totten/civix#189 that fixes an issue with autogenerated upgrader

[eileenmcnaughton]

As with the other one - let's close this & just keep the parts that are reviewable open. They don't need to be open to be used as a to-do for the person working on them but I'm trying to get the PR list down to being a review to-do list

[eileenmcnaughton]

Oh I just saw your comment - I think the same thing can be achieved by having a branch though? I've put about 70 hours into triaging & reviewing just recently & I'm very focussed on how to make the PR list work as a review to-do list at the moment