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

SearchKit - Allow smarty in field rewrite #22592

Merged
merged 1 commit into from
Jan 26, 2022

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented Jan 21, 2022

Overview

This gives the ability to use Smarty syntax in the "rewrite" of SearchKit displays.

Before

Tokens like [first_name] supported.

After

Those tokens still work and will be evaluated first. They can be mixed in with Smarty tags like {if "[first_name]"}.

Technical Details

Smarty will see the evaluated values for the tokens (which should be quoted if string and inside a smarty tag).

Note: Due to #1371, PHP filters like strtolower or ucfirst cannot be used.

Comments

This is #22346 without assigning the variables, which @eileenmcnaughton recommended against.

@civibot
Copy link

civibot bot commented Jan 21, 2022

(Standard links)

civicrm_api4('SearchDisplay', 'run', $params);
$this->fail();
}
catch (\Exception $e) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What exception does it catch here?

Copy link
Member Author

@colemanw colemanw Jan 21, 2022

Choose a reason for hiding this comment

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

I don't really understand the mechanics of it, but smarty calls trigger_error and somehow that's catchable in the unit test, but I wasn't able to do much with it. $e->getMessage() returns an empty string.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh right THAT error - yeah it is inconsistently escalated - there has been discussion before about consistently escalating to being a 'real live error' - it kinda got too hard to do it for the code we currently use because it skipped the scope of the deprecated classes - but I think it would be worth wrapping & escalating your fetch with an error handler like

https://github.com/civicrm/civicrm-core/pull/21122/files#diff-3f93091ccbe99a7c1ae88473057539b1021099b37852cfea68ff948f9f8d1a86R134-R136

so that we don't create a new place that behaves inconsistently across the different CMS

Copy link
Contributor

Choose a reason for hiding this comment

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

note I think the handler would go on CRM_Core_Smarty

Copy link
Member Author

Choose a reason for hiding this comment

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

@eileenmcnaughton is that a blocker on this PR?

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

Successfully merging this pull request may close these issues.

2 participants