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

Fix locale leaking into tokens #26179

Merged
merged 1 commit into from
Jun 2, 2023

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Fix locale leaking into tokens

Before


I'm trying to convert some code that uses our own message template stuff to call the core CRM_Core_BAO_MessageTemplate::render() function - while this isn't an api we don't have an equivalent at the moment & as our code is heavily tested I feel OK taking the risk calling it from outside of core. (Whether I can figure out the parameters is TBD)

However, what I'm finding is that I have

  protected function exportExtraTokenContext(array &$export): void {
    $export['smartyTokenAlias']['first_name'] = 'contact.first_name';

When the name is NOT provided the call to pathGet is using the default it is passed .... the locale - so I wind up with 'Dear en_US'

After

I get Dear as I would expect when first_name is not provided

Technical Details

I'm still deep in the struggling part of this bit of work so I struggle to articulate much but looking at the function signature of pathGet I can't see any reason why the locale rather than an empty string would be passed. I suspect NULL is OK too - but I'm scared to use NULL within token code code it has a history of breaking things

Comments

@totten

@civibot
Copy link

civibot bot commented May 9, 2023

(Standard links)

@civibot civibot bot added the master label May 9, 2023
@demeritcowboy
Copy link
Contributor

@eileenmcnaughton It looks like you added it at #21783 but I think it was a typo and you meant to add it as the 3rd parameter to \Civi::format()->money() a few lines down.

So I'd say it's mergeable as-is, but I wonder if it should be added to money.

@demeritcowboy demeritcowboy added the merge ready PR will be merged after a few days if there are no objections label Jun 1, 2023
@eileenmcnaughton
Copy link
Contributor Author

@demeritcowboy yes - you might be right about the intent - I'll look at that again in a bit so I think this can go through as is but it is something for me to think about

@demeritcowboy demeritcowboy merged commit f811aaa into civicrm:master Jun 2, 2023
@eileenmcnaughton eileenmcnaughton deleted the token_get branch June 2, 2023 03:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master merge ready PR will be merged after a few days if there are no objections
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants