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

Replace smarty fetch() call with CRM_Utils_String::parseOneOffStringThroughSmarty() #53

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

svenow
Copy link

@svenow svenow commented Feb 24, 2025

Replaced smarty fetch() call with CRM_Utils_String::parseOneOffStringThroughSmarty

systopia-reference: 27391

@svenow svenow self-assigned this Feb 24, 2025
@svenow svenow requested a review from jensschuppe February 24, 2025 13:04
Copy link
Collaborator

@jensschuppe jensschuppe left a comment

Choose a reason for hiding this comment

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

The point of using the new method is to make instantiation of the CRM_Core_Smarty class obsolete, as it is being instantiated inside that new method again.

@svenow
Copy link
Author

svenow commented Feb 25, 2025

i see. will add some comments with further questions

@@ -163,7 +163,7 @@ public static function validateSmarty($smartyValue) {

Copy link
Author

@svenow svenow Feb 25, 2025

Choose a reason for hiding this comment

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

i guess, this can be removed as well as it would only be invoked in $smarty->fetch() and parseOneOffStringThroughSmarty (as seen in the code) registers its own error handler?

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is being referenced as a form rule here:

$this->registerRule('is_valid_smarty', 'callback', 'validateSmarty', 'CRM_Moregreetings_Form_Settings');

So the rendering within that method should be adjusted as well (or refactored to use a single render method)

…fStringThroughSmarty

removed obsolete smarty occurences and related code

increased version of civicrm docker image to 5.75 in order to be able to use CRM_Utils_String::parseOneOffStringThroughSmarty
@svenow svenow force-pushed the replace-smarty-fetch branch from 62620c4 to af7c691 Compare February 25, 2025 11:10
@svenow svenow requested a review from jensschuppe February 25, 2025 11:14
rephrased comment about necessary civicrm version in phpunit workflow file
@svenow svenow requested a review from jensschuppe February 25, 2025 12:26
@jensschuppe jensschuppe added bug status:needs review Code needs review and testing labels Feb 25, 2025
@jensschuppe jensschuppe added this to the 1.2 milestone Feb 25, 2025
@jensschuppe
Copy link
Collaborator

jensschuppe commented Feb 25, 2025

With enough test feedback this should go into a 1.3.x release.

@jensschuppe jensschuppe changed the title replace obsolete smarty fetch call with CRM_Utils_String::parseOneOff… Replace smarty fetch() call with CRM_Utils_String::parseOneOffStringThroughSmarty() Feb 25, 2025
@jensschuppe jensschuppe modified the milestones: 1.2, 1.3 Feb 25, 2025
@jensschuppe
Copy link
Collaborator

@svenow for tests, please rebase onto the current master with #55 being merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug status:needs review Code needs review and testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Apply Smarty Security Policy and replace usage of {crmAPI} in affected templates
2 participants