Standalone - check logged in User's locale in multilingual mode

[ufundo]

Update

Following #32525 this now just contains the implementation of getUFLocale for Standalone to read the logged in User's locale setting as part of the applyLocale cascade.

This behaviour is controlled by the inheritLocale setting.

Overview

Attempt to unbreak multilingual on Standalone.

Before

• in the system bootstrap, UF postContainerBoot is the first thing that runs after the container has booted
• in Standalone, this initiates the user session and sets user timezone
• if you enable multilingual, it causes a crash because it tries to load the user data (to set the timezone) before any locale is set
• none of the other UFs do anything in the postContainerBoot hook
• inheritLocale is disabled on Standalone, so never checks user language
• getUFLocale isn't implemented for Standalone, so can't check user language

After

• move postContainerBoot two steps later: after Settings is fully booted (can load db values) and after applyLocale is called
• move setTimeZone call later => bootstrap doesnt crash when enabling multilingual
• potential bad consequences of setting user timezone/session later? => seem ok from testing, but it slightly worries me that another extension's hook_civicrm_config could come in earlier, before the timezone is set (note: this would be the case already on other UFs, so hook_civicrm_config shouldn't expect timezone as it stands)
• remove block on setting inheritLocale on standalone, make wording reference User rather than CMS
• implement the getUFLocale check => note this requires the session is initialised, suggesting postContainerBoot should come before applyLocale. There's also a note about temporarily setting dbLocale before checking getUFLocale, which sounds like related issue to the initial crash

Comments

Is there any reason postContainerBoot is exactly where it is? I remember in the work to get Standalone stable there used to be a crash in applyLocale, though I can't remember exact details, or if it had any interaction with postContainerBoot, and so that was an important constraint. But looking at the commit history, I suspect it's just "where it always was".~~

It would be good to retest the various bugs we found in Standalone timezone handling (PHP + MySQL) for any regressions on that front.

And see what Jenkins says.

[civibot]

🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷

Introduction for new contributors...

• If this is your first PR, an admin will greenlight automated testing with the command ok to test or add to whitelist.
• A series of tests will automatically run. You can see the results at the bottom of this page (if there are any problems, it will include a link to see what went wrong).
• A demo site will be built where anyone can try out a version of CiviCRM that includes your changes.
• If this process needs to be repeated, an admin will issue the command test this please to rerun tests and build a new demo site.
• Before this PR can be merged, it needs to be reviewed. Please keep in mind that reviewers are volunteers, and their response time can vary from a few hours to a few weeks depending on their availability and their knowledge of this particular part of CiviCRM.
• A great way to speed up this process is to "trade reviews" with someone - find an open PR that you feel able to review, and leave a comment like "I'm reviewing this now, could you please review mine?" (include a link to yours). You don't have to wait for a response to get started (and you don't have to stop at one!) the more you review, the faster this process goes for everyone 😄
• To ensure that you are credited properly in the final release notes, please add yourself to contributor-key.yml
• For more information about contributing, see CONTRIBUTING.md.

Quick links for reviewers...

• ➡️ PR Demo site (can be rebuilt with the command test this please).
• 📖 Review standards
• 🗒️ Review template (brief or verbose)

➡️ Online demo of this PR 🔗

[ufundo]

FYI @seamuslee001 this seems to work from local testing but could definitely do with some more interrogation

[colemanw]

@seamuslee001 this patch is smaller. Does yours cover anything extra though that this one misses?

[seamuslee001]

@colemanw @ufundo I haven't looked at what standalone does off the back of the Config::singleton() call.

I would note that for both WordPress and Drupal8 They set the TimeZone well after everything in CRM_Core_Config is done.

In my other PR I did it that way as I figured that the order or the steps here was important for Standalone as I thought that was what you were saying on Thursday/Friday @ufundo in the meeting.

[ufundo]

In my other PR I did it that way as I figured that the order or the steps here was important for Standalone as I thought that was what you were saying on Thursday/Friday @ufundo in the meeting.

That was my memory @seamuslee001 .. but looking at it again I couldn't see the dependency.

I think maybe there was some other issue with applyLocale before that made the order more sensitive... but it seems fine to swap it now.

[ufundo]

From testing on the demo site, I've been able to:

• set a different timezone for demo user
• create new contribution (time seems good)
• create new note (time seems good)
• enable multilingual
• add German
• visit in german using lcMessages param ( http://core-32444-nnscsa.test-1.civicrm.org:8003/civicrm/home?lcMessages=de_DE )
• set some German option group value

I tried setting my user language preference to German, and that doesn't seem to work yet.

[seamuslee001]

@ufundo i think the language preference on the civicrm_contact record might only be used for things like schedule reminders. Maybe we need to have a locale field or something on the uf_match (user) table to say this logged in user should be in locale y or something?

I also feel like needing to use the lcMessages url param feels not brilliant but do accept at the moment it is the way to switch locales

@mlutfy do you have any thoughts on locale handling in standalone? I

[ufundo]

@seamuslee001 there is already a language option on the Standaloneusers user record.

I'm looking at bit more now and there are a few hurdles:

1. standalone currently freezes the inheritLocale option, so it never tries to get the user's language => easy fix
2. then we need to implement getUFLocale on the CRM_Utils_System_Standalone => also pretty easy
3. then there's a bootstrap issue => getting the getUFLocale checks for logged in user, which doesn't work until the session is initialised, which currently only happens in the postContainerBoot, but we just moved that after applyLocale . Having a play with ways round this now..

On the basis of 3, I'm not sure if this fix is right.

[mlutfy]

I don't have much to add here, but just wanted to double-check:

there is already a language option on the Standaloneusers user record.

I guess one annoying thing is that a user will have a default language preference (on their user record), and may also want to temporarily change their UI preference (using a language switcher, and therefore the lcMessages URL param).

For example, some features in CiviCRM depend on the language of the UI (PDF Letters / Send Mail, with tokens, for example, and most likely receipts). There are also some fields/settings in multi-lingual that are only translatable if you change the entire UI.

[seamuslee001]

@ufundo I don't like changing this because for WP/Joomla/BackDrop/Drupal8 we are inheriting from the CMS not the user setting (User setting may affect what the CMS sets)

[seamuslee001]

@mlutfy what do you think about the language changes here?

[ufundo]

Ah. I was thinking it was the User either way (Standalone user or CMS user)... hadn't thought the CMS could decide on language another way.

"Inherit CMS/User Language"?

I think there's a few places like this where the ideal would be to override the wording on Standalone - I think that should be possible using a hook on settings meta, but I haven't quite pinned it down.

[seamuslee001]

In WordPress / Drupal the default method CMS sets the locale especially for anonymous users is via the url path e.g /en/ or /fr/ etc.

[ufundo]

Have updated to use alterSettingsMetaData hook in standaloneusers, so the wording is only changed on Standalone 👍

[ufundo]

I guess one annoying thing is that a user will have a default language preference (on their user record), and may also want to temporarily change their UI preference (using a language switcher, and therefore the lcMessages URL param).

Language setting from the user is happening the same way it does on the CMSes. The inheritLocale mentions a "CiviCRM Language Switcher" - so that should still work the same way it does on the CMS (I don't know what that is or how it works)

there is already a language option on the Standaloneusers user record.

One thing I was wondering is why we have a language option on the User record, given there is also a preferred language on the Contact record for that user. I used the value from the User record because it is there. But maybe we could get rid of that and just use the Contact value? (I don't know if there are times you might when either Someone might want a different value on their Contact / User records; or if as a site-builder you might want different options available on Contact record vs User record?)

[ufundo]

Update: it looks like language is on UFMatch and so isn't introduced by Standalone. In which case removing it is harder. So prob I'd guess we leave it. Maybe getUFLocale could check the Contact language if the User language is not set?

At any rate, I don't think it would be out of in scope of this PR.