-
-
Notifications
You must be signed in to change notification settings - Fork 823
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
Smarty 3/4 - Fix prepending extension template directories #30148
Conversation
🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷 Introduction for new contributors...
Quick links for reviewers...
|
Needed to support civicrm/civicrm-core#30148
Before: Extensions would prepend their directories under smarty2, but smarty3 & 4 would append them, breaking template overrides. After: Smarty 2,3&4 all prepend template directories Fixes civicrm/org.civicrm.contactlayout#143
79554c5
to
2b2dfac
Compare
Since this is specifically aimed at the various edges of Smarty (2/3/4/5), I ran the (There were php82 warnings, esp w/smarty2+3+5, but I believe they're all pre-existing.) I'll do one more check to with civix using this in a backport... |
OK, yup, backport loads fine with civix on an older version of CiviCRM. |
Smarty 3/4 - Fix prepending extension template directories
Just noting that in our experience |
I performance tested - if it wasn't for the first line in this table this would be in my sights ... https://lab.civicrm.org/dev/core/-/issues/5243#note_164882 |
Ok - this does seem to be the culprit for the slow down - but we have been firing up some variants that address it & we can look at the pre-existing issue once Montreal is online |
Overview
Fixes civicrm/org.civicrm.contactlayout#143
Before
Extensions would prepend their directories under smarty2, but smarty3 & 4 would append them, breaking template overrides.
After
Smarty 2,3&4 all prepend template directories
Technical Details
It was a bit tough to wrangle the same behavior out of all 3 versions, but resorting to lowest-common-denominator getter/setter functions works well.Note: I ended up adding a special case for Smarty2 since it doesn't have getter/setter functions (at least not on older versions of Civi, which we still support via mixin shims). So this PR no longer depends on civicrm/civicrm-packages#397