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

[REF] APIv4 - Dispatch event during Entity.get #21803

Merged
merged 1 commit into from
Oct 13, 2021

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented Oct 12, 2021

Overview

This allows extensions to modify the list of entities, enabling "virtual" entities not based on php files.
Followup to #21771

Before

Assumptions made about entities belonging in files.

After

Entity retrieval and caching refactored to call an event.

Technical Details

I had to refactor the part that fetches custom groups because using the api within API.entity.get was causing recursion.

Comments

I submitted this against 5.43 because #21771 is in 5.43 and it would be best to have them together.

@civibot
Copy link

civibot bot commented Oct 12, 2021

(Standard links)

@civibot civibot bot added the 5.43 label Oct 12, 2021
@colemanw colemanw force-pushed the APIv4EntityEvent branch 2 times, most recently from d70d040 to 4d5845f Compare October 12, 2021 12:36
@eileenmcnaughton
Copy link
Contributor

image

@eileenmcnaughton
Copy link
Contributor

I wonder is this PR will help with the apiv4 getfields caching bug that has stymied me here #21790

@colemanw
Copy link
Member Author

@eileenmcnaughton yea, the functional change here is that previously there were 2 incomplete caches - a cache of entity classes, and a cache of (partial) entity info. It was lazier that way but not necessarily more efficient. This new way is IMO better - it does the whole scan once and caches a single array of all entities and their complete info.

@eileenmcnaughton
Copy link
Contributor

@colemanw style

File Priority Age Author Commit ID
BasicActionsTest.php:35

This allows extensions to modify the list of entities,
enabling "virtual" entities not based on php files.
@colemanw
Copy link
Member Author

colemanw commented Oct 12, 2021

@colemanw style

Grr, it's that stupid "Data types in @param tags need to be fully namespaced" warning. IMO we should get rid of that; PHPStorm agrees with me.

@eileenmcnaughton
Copy link
Contributor

Tests are passing and they passed against our test suite too

@@ -26,6 +26,7 @@ public function setUp(): void {
'created_date' => 'first sat of July 2008',
];
parent::setUp();
\CRM_Core_BAO_ConfigSetting::enableComponent('CiviCampaign');
Copy link
Member Author

Choose a reason for hiding this comment

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

Yea, this failed because CampaignTest neglected to enable the CiviCampaign component!

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

Successfully merging this pull request may close these issues.

2 participants