CRM_Core_I18n::setLocale should change $tsLocale for getLocale() to work correctly

[mlutfy]

Overview

Adds a test for getLocale/setLocale, which I suspect is not working as intended, since setLocale does not change the tsLocale, but getLocale reads from it (it still changes the UI strings, because it changes the gettext configuration, but other things such as smarty templates checking against tsLocale might be confused).

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[mlutfy]

The test fails, as expected:

CRM_Core_I18n_LocaleTest::testI18nLocaleChange
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'en_US'
+'fr_CA'

/home/jenkins/bknix-dfl/build/core-13050-v1qx/sites/all/modules/civicrm/tests/phpunit/CRM/Core/I18n/LocaleTest.php:52
/home/jenkins/bknix-dfl/build/core-13050-v1qx/sites/all/modules/civicrm/tests/phpunit/CiviTest/CiviUnitTestCase.php:192
/home/jenkins/bknix-dfl/civicrm-buildkit/extern/phpunit5/phpunit5.phar:598

I'll add another commit that should fix it.

[mattwire]

@mlutfy Is there a simple way for me to test this in the UI? What would I expect to see?

[mlutfy]

Not easily. One use-case I had run into was with cdntaxreceipts, when I edited the cdntaxreceipt message template with, for example:

{if $tsLocale == "fr_FR"}
Bonjour,
{else}
Hello,
{/if}

(this is not a great practice, an alternative is to use {ts domain="mycustomextension"}, store a copy of the msg-template in it, and use civistrings to generate a gettext file that you can translate.. but that can be a lot of work)

and then:

• set your default site language to English, enable multi-lingual, add French
• create a contact whose language pref is fr_FR
• set your interface UI to English
• Generate a cdntaxreceipt for that contact, using the backend

Normally the cdntaxreceipt extension should setLocale to fr_FR, so that it generates a receipt in French for that contact. The PDF will be in French, but the email with the text based on tsLocale will be in English.

I was working on:

• https://github.com/jake-mw/CDNTaxReceipts/pull/86
• https://civicrm.stackexchange.com/questions/27074/contribution-page-validation-errors-display-and-confirmation-screen-switch-to-de

I was encountering situations where 3rd-party extensions, such as cdntaxreceipts, were generating UIs that are part English, part French.

setLocale isn't used much in CiviCRM. It mostly has two use-cases: when sending mailings in another language (on multilingual sites), and for cdntaxreceipts (although it should be used in more places, but it's somewhat recent, and users have the habit of working around those limitations, such as by switching the interface language when communicating to a user in their language).

setLocale would correctly switch the gettext language and db language, but by not changing the tsLocale, Smarty templates that were doing conditionals on that variable would not evaluate correctly.

[eileenmcnaughton]

This is a simple change and it's well explained and makes a lot of technical sense.

The global $tsLocale var seems a bit smelly but since we use it making it right makes sense.

We have just cut an rc today so merging this now gives it 2 full months before release which seems good since I don't feel any testing we do is particularly meaningful. Merging