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

Allow custom ts functions in extensions; defer custom ts calls until booted #15411

Merged
merged 4 commits into from
Oct 7, 2019

Conversation

bjendres
Copy link
Contributor

@bjendres bjendres commented Oct 6, 2019

Overview

Custom ts functions didn't work when they are defined by extensions. The reason is that the I18n class starts looking for it when the extension files aren't required yet, and then gives up before they become available.

Before

If you set a custom ts function (CRM_Core_Config::singleton()->customTranslateFunction) that is defined in the module.php file in an extension, it wouldn't work.

After

It does work.

Technical Details

Changes discussed with @totten:

  • consider the bootstrap process to be progressed enough to find the custom ts function after \Civi\Core\Container::isContainerBooted() instead of \Civi\Core\Container::getBootService('settings_manager')
  • don't translate any strings before the bootstrap process has reached that stage, in order to avoid double initialisation of the gettext system.
  • renamed $areSettingsAvailable to $bootstrapReady for clarity.
  • added skip_translation flag to ts() parameters
  • avoid spinning up gettext during bootstrap (in case there is a custom ts function)

Comments

Anything else you would like the reviewer to note

@civicrm-builder
Copy link

Can one of the admins verify this patch?

@civibot
Copy link

civibot bot commented Oct 6, 2019

(Standard links)

@civibot civibot bot added the master label Oct 6, 2019
@seamuslee001
Copy link
Contributor

Jenkins add to whitelist

@totten
Copy link
Member

totten commented Oct 6, 2019

The build failure is relevant - i.e. indicating that ts() is used as part of the SQL-generation logic for a new install, which necessarily happens before the system has bootstrapped.

@totten totten changed the title implemented @totten's approach to fix custom ts functions in extensions (WIP) Allow custom ts functions in extensions; defer custom ts calls until booted Oct 6, 2019
@seamuslee001
Copy link
Contributor

Jenkins re test this please

@seamuslee001
Copy link
Contributor

Changes make sense to me and the fact the build site runs is a good sign this PR is working.

@seamuslee001 seamuslee001 changed the title (WIP) Allow custom ts functions in extensions; defer custom ts calls until booted Allow custom ts functions in extensions; defer custom ts calls until booted Oct 7, 2019
@seamuslee001
Copy link
Contributor

(CiviCRM Review Template WORD-1.2)

  • General standards
    • (r-explain) Pass
    • (r-user) Pass
    • (r-doc) Pass
    • (r-run) Pass i created a custom translation function in an extension and then enabled the extension and set the funtion in the setting and confirmed that the strings were appropriately modified
  • Developer standards

@seamuslee001 seamuslee001 merged commit 7f58fa4 into civicrm:master Oct 7, 2019
bjendres pushed a commit to systopia/civicrm-core that referenced this pull request Oct 9, 2019
Allow custom ts functions in extensions; defer custom ts calls until booted
bjendres pushed a commit to systopia/civicrm-core that referenced this pull request Oct 9, 2019
totten added a commit to totten/civicrm-core that referenced this pull request Jan 23, 2020
Overview
--------

This fixes a regression in which seed data is not translated.

Before
------

The `civicrm_data.*.mysql` files are not translated.

After
-----

The `civicrm_data.*.mysql` files are translated.

There's a unit-test to check this.

Comments
--------

This regression stems from civicrm#15411, which aimed to allow extensions to define
custom variants of `ts()`.  The crux of the issue is "What happens if you
try to translate a string before the system is bootstrapped - before the
extension is loaded?  What's your fallback behavior?"

In civicrm#15411, it used a fallback behavior of "do no translation".  In theory,
you shouldn't really get into this scenario since UIs are pretty much always
generated post-boot.

However, it turns out that there is a situation where you have an un-booted
system and need to translate strings -- i.e. when generating the localized
`civicrm_data.*.mysql` data.  Hence the bug.

This patch preserves most of the changes from civicrm#15411, but it changes the
fallback behavior from "do no translation" to "use the built-in/default
translator".
totten added a commit to totten/civicrm-core that referenced this pull request Jan 23, 2020
Overview
--------

This fixes a regression in which seed data is not translated.

Before
------

The `civicrm_data.*.mysql` files are not translated.

After
-----

The `civicrm_data.*.mysql` files are translated.

There's a unit-test to check this.

Comments
--------

This regression stems from civicrm#15411, which aimed to allow extensions to define
custom variants of `ts()`.  The crux of the issue is "What happens if you
try to translate a string before the system is bootstrapped - before the
extension is loaded?  What's your fallback behavior?"

In civicrm#15411, it used a fallback behavior of "do no translation".  In theory,
you shouldn't really get into this scenario since UIs are pretty much always
generated post-boot.

However, it turns out that there is a situation where you have an un-booted
system and need to translate strings -- i.e. when generating the localized
`civicrm_data.*.mysql` data.  Hence the bug.

This patch preserves most of the changes from civicrm#15411, but it changes the
fallback behavior from "do no translation" to "use the built-in/default
translator".
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants