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

CRM_Utils_Date - Month and day names should match active locale #21569

Merged
merged 2 commits into from
Sep 22, 2021

Conversation

totten
Copy link
Member

@totten totten commented Sep 22, 2021

Overview

(Note: Some of the unit-tests in this patch depend on items from #21531. The PR will look a lot smaller after merge+rebase.)

This fixes a cache-bug in CRM_Utils_Date which prevents dates from being localized, as in a scenario like:

CRM_Core_I18n::singleton()->setLocale('es_MX');
echo CRM_Utils_Date::customFormat(... '...%B...' ...);
CRM_Core_I18n::singleton()->setLocale('fr_FR');
echo CRM_Utils_Date::customFormat(... '...%B...' ...);

Before

The first time that it translates a month name (%B), CRM_Utils_Date stores a cache of all month names. This is shared by all locales. Thus, a French-speaking user may wind up seeing dates with Spanish names.

After

The cache of month names and date names is split (per-locale).

Technical Details

I included a lower-level test to show the various caches are locale-friendly - as well as a higher-level test to show that dates are localized when composing batch-messages for different people in different locales.

@civibot
Copy link

civibot bot commented Sep 22, 2021

(Standard links)

@civibot civibot bot added the master label Sep 22, 2021
@eileenmcnaughton
Copy link
Contributor

eileenmcnaughton commented Sep 22, 2021

@totten do you want to rebase this now as the other is merged

Also - I haven't tested timezones yet. I can think of these use cases

  1. "Document printed at {domain.now} - you would want your user tz probably. If sharing internationally you can add timezone to the output
  2. Contribution received on 20-Jan-2020 - this value isn't timezone aware in the DB but is recorded in the 'server timezone'. I feel like we expect people to get what the server holds.

Ug and just like that every cell in my body is on strike unless I find coffee....

UPDATE - we discussed this & I think that current behaviour is no change once I configure the system tz correctly. We did also note

  1. civicrm_address has a timezone field
  2. ideally crmDate would accept timezone in some way - but we can push out of scope...

@eileenmcnaughton
Copy link
Contributor

oh lol - done

@eileenmcnaughton
Copy link
Contributor

With this rebased it's pretty clear it's just caching fixes

@totten
Copy link
Member Author

totten commented Sep 22, 2021

@eileenmcnaughton Yup, just rebased.

Agree timezones are a good topic, but I think that'll be more difficult. The question there is: How do you know the contact's preferred timezone? AFAIK, Civi hasn't been tracking that on a per-contact-basis - instead we try to integrate with other sources of preferred TZ. That seems like it'll need some layers to untangle...

For the interim, we could put some energy into better communicating TZs -- e.g. in the docs+defaults, provide formatting examples that display the TZ...

@eileenmcnaughton eileenmcnaughton merged commit 5390a21 into civicrm:master Sep 22, 2021
@totten totten deleted the master-datets branch September 22, 2021 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants