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

totten/civix#257 Fix Civix not correctly loading Mixinx #25179

Merged
merged 1 commit into from
Dec 16, 2022

Conversation

jaapjansma
Copy link
Contributor

Overview

This is basically a fix for Civix. Civix had a mistake in the generated boilerplate causing to run the mixin only for the first installed extension.

Before

Mixin functions where only called once. So when a CiviCRM installation has multiple extensions the mixin functions where only called for one. Resulting in breaking functionality, such as not loading the xml/Menu/*.xml files.

After

The mixin functions are called for every extension. It loads it only once for each extension.

Comments

This is a fix to the boiler template used by Civix.

See as well: totten/civix#257

@civibot
Copy link

civibot bot commented Dec 15, 2022

(Standard links)

@demeritcowboy
Copy link
Contributor

jenkins retest this please

@demeritcowboy
Copy link
Contributor

I think something is wrong with the test nodes.

@totten
Copy link
Member

totten commented Dec 15, 2022

Nice find, @jaapjansma! The analysis and patch look compelling. In its current context, __FUNCTION__ resolves to the non-unique expression {closure}. (Apparently, anonymous classes get unique __CLASS__ names, but anonymous functions don't get unique __FUNCTION__ names.)

We just need to get through testing.

(@demeritcowboy) I think something is wrong with the test nodes.

Yeah, I've noticed some similarly weird results recently on other PRs. Taking a look...

@seamuslee001
Copy link
Contributor

Jenkins re test this please

@eileenmcnaughton eileenmcnaughton changed the title Fix for CiviX issue 257 Fix Civix issue not correctly loading Mixinx - totten/civix#257 Dec 16, 2022
@eileenmcnaughton eileenmcnaughton changed the title Fix Civix issue not correctly loading Mixinx - totten/civix#257 totten/civix#257 Fix Civix not correctly loading Mixinx Dec 16, 2022
@totten
Copy link
Member

totten commented Dec 16, 2022

I've run the civix test-suite using the updated polyfill.php -- and deployed the extensions on top of 5.44 (where the polyfill is still needed). Confirmed the bug (before patch), and confirmed the fix (after patch). 👍

Looks good. Merging.

Related: totten/civix#279

@totten totten merged commit eeae05a into civicrm:master Dec 16, 2022
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.

4 participants