Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dev/core#3961 - Move component Api4 files to component extensions #26208

Merged
merged 2 commits into from
Jun 8, 2023

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented May 12, 2023

Overview

Following on #26036 this is a very small step toward moving component code into extensions: this moves the APIv4 classes.
See dev/core#3961 proposal to create a stub extension for all components.

Before

Component api files in core.

After

Component api files in component extensions

Technical Details

This moves the code belonging to components into their extension directories. As before, these apis will only be available if the component is enabled, but now there is no extra code needed to make that happen.

@civibot
Copy link

civibot bot commented May 12, 2023

(Standard links)

@civibot civibot bot added the master label May 12, 2023
@colemanw colemanw force-pushed the extensionComponentApi branch from 8274d50 to d616e55 Compare May 12, 2023 20:32
@colemanw colemanw changed the title Apiv4 - Move component api files to component extensions dev/core#3961 - Move component Api4 files to component extensions May 12, 2023
@colemanw colemanw marked this pull request as draft May 12, 2023 20:40
@totten
Copy link
Member

totten commented May 13, 2023

Related discussion: https://chat.civicrm.org/civicrm/pl/159e3dzuw7yuxj1zxqjg6zx5pw

As before, these apis will only be available if the component is enabled, but now there is no extra code needed to make that happen.

Right, I see what you're saying. That's the linchpin.

Do any more edges come to mind if you think about:

  • Metadata bits like ::permissions() or ::getFields()?
  • For these APIs, was it always necessary to have the component enabled? I thought that had changed somewhere in the past year?

--

FiveSixtyOne::convertMappingFieldsToApi4StyleNames() fails when it calls Civi\API\Contribution::getFields() during an upgrade.

I bet this is failing because of the order-of-operations. At the time when FiveSixtyOne executes, the civicrm_extension[full_name=civi_contribute] is not installed. So naturally FiveSixtyOne shouldn't have access to resources from ext/civi_contribute/. At best, maybe those resources become accessible after FiveSixtyThree.

Perhaps the issue would be moot if FiveSixtyOne used low-dependency code - maybe:

$fieldMap += CRM_Core_DAO::executeQuery('
  SELECT CONCAT("custom_", fld.id) AS old, CONCAT(grp.name,".",fld.name) AS new 
  FROM civicrm_custom_field fld...
')->fetchMap('old', 'new');

@colemanw
Copy link
Member Author

Thanks @totten I've updated the upgrader per your suggestion. To your other questions:

  • I can't think of any gotchas with metadata bits
  • APIv4 has excluded entities from disabled components since d31fb4e

// Ensure all components are enabled so their entities show up
foreach (array_keys(\CRM_Core_Component::getComponents()) as $component) {
\CRM_Core_BAO_ConfigSetting::enableComponent($component);
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was redundant with the setUp function in this class which calls enableAllComponents()

@colemanw colemanw force-pushed the extensionComponentApi branch from 5287f33 to 7323840 Compare May 15, 2023 17:37
colemanw added a commit to colemanw/civicrm-core that referenced this pull request May 16, 2023
This was causing issues with civicrm#26208
because the Contribution API is no longer available without enabling the
civi_contribute extension, which doesn't happen until a later upgrade step.
colemanw added a commit to colemanw/civicrm-core that referenced this pull request May 16, 2023
This was causing issues with civicrm#26208
because the Contribution API is no longer available without enabling the
civi_contribute extension, which doesn't happen until a later upgrade step.
@colemanw colemanw force-pushed the extensionComponentApi branch from 7323840 to 53af82a Compare May 17, 2023 03:33
This moves the code belonging to components into their extension directories. As before, these apis will only be available if the component is enabled, but now there is no extra code needed to make that happen.
@colemanw colemanw force-pushed the extensionComponentApi branch from 53af82a to ba2af25 Compare June 7, 2023 22:53
@colemanw colemanw marked this pull request as ready for review June 7, 2023 22:54
@totten
Copy link
Member

totten commented Jun 7, 2023

Yup, we've just started the cycle for 5.64.alpha - seems like a good time to get this in. Merge on pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants