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

Entity types hook recursion fix #21851

Merged
merged 2 commits into from
Oct 19, 2021

Conversation

colemanw
Copy link
Member

Overview

Follow-up to #21828 for avoiding recursion when subscribing to the new civi.api4.entityTypes event and calling the API within the subscriber

Before

Uses API in a function called by APIv4 Entity.get. Recursion.

After

Cleaned up to fix a fixme and not call the api recursively.

jensschuppe and others added 2 commits October 18, 2021 08:02
Use plain SQL instead of APIv4 in a function that is called by APIv4 Entity.get
to avoid an infinite loop.
This fixes the fixme in basicTypeInfo to use getAllContactTypes instead of running its
own query, and cleans up some comments.
@civibot
Copy link

civibot bot commented Oct 18, 2021

(Standard links)

@eileenmcnaughton
Copy link
Contributor

@jensschuppe can you confirm this addresses the issue you hit?

@jensschuppe
Copy link
Contributor

@jensschuppe can you confirm this addresses the issue you hit?

@eileenmcnaughton Yes, I can confirm.

@colemanw
Copy link
Member Author

@eileenmcnaughton this also fixes a fixme you left in the comments :)

@eileenmcnaughton
Copy link
Contributor

@colemanw I'm happy to merge this - but I kinda wonder about casting those strings back to null?

@colemanw
Copy link
Member Author

colemanw commented Oct 19, 2021

@eileenmcnaughton the reason I removed that bit is because I thought it was over-zealous casting. There's no practical difference.

@eileenmcnaughton eileenmcnaughton merged commit 617f004 into civicrm:5.43 Oct 19, 2021
@eileenmcnaughton
Copy link
Contributor

ok

@eileenmcnaughton eileenmcnaughton deleted the entityTypesHookRecursionFix branch October 19, 2021 19:45
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.

3 participants