Scheduled Reminders - Pass locale through to TokenProcessor #21085
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This can currently be done with `swap()`, but we may be doing this several times, and `swapLocale()` is a little more pithy and a little more microoptimal.
…cale(pickLocale(...))
There are two likely ways in which a tokenized email winds up with localized strings - either
the Smarty `{ts}...{/ts}` tags define it, or a custom/hook-based tag uses `ts()`.
This ensures that the locale is set in both cases.
It's hypothetically possible that some other `civi.token.eval` listeners
need to use the `row->context['locale']`. However, I grepped universe and
couldn't find anything that would be affected. (There were two contrib
listeners for `civi.token.eval` and neither seemed to be affected.)
…le']` Before: Runs `setLocale()` and then executes the entire pipeline for `TokenProcessor` After: Leaves the global locale alone. Instead, rely on `TokenProcessor` to switch locale as it visits each recipient.
From a core POV, this should be unnecessary. And I'm not sure that it's a good idea for contrib to assume that the locale has been set to the email receipient. (It probably isn't done that way in some contexts.) Never-the-less, this should provide greater continuity with the pre-existing behavior. If anyone, say, uses `Hook::alterMail()` to append a translated footer, then it should continue to work in the same way as before.
|
(Standard links)
|
|
This seems straight forward & sensible & the test gives confidence |
seamuslee001
added a commit
to JMAConsulting/ca.civicrm.moneris
that referenced
this pull request
Jan 28, 2022
seamuslee001
added a commit
to JMAConsulting/ca.civicrm.moneris
that referenced
this pull request
Jan 28, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Update the mechanism by which Scheduled Reminders (
ActionSchedule) activate the appropriate locale.In a subsequent PR, this will allow significant simplification/maintainability improvements for the
CRM_*_Tokensadapters.Before
The
ActionSchedulecontroller callsI18n->setLocale()directly. It then creates a separateTokenProcessorfor each recipient and runs the fullTokenProcessorroutine.After
The
ActionSchedulecontroller does not callI18n->setLocale()directly. Instead, it passes thelocaleto theTokenProcessor. At key moments (while handling each recipient/message), it will switch the active locale.Technical Details
There are a couple ways for a translated string to get into a scheduled-reminder -- most likely by way of Smarty expression (
{ts}...{/ts}). There are a couplehook_civicrm_tokenValuesconsumers inuniversewhich callts(). At a stretch, it's hypothetically possible thathook_civircm_alterMailmight inject a translated string. This patch should give the same outcomes in all those cases.