Page MenuHomePhabricator

Contact updated to have an invalid language
Closed, ResolvedPublic

Description

select * FROM log_civicrm_contact WHERE id = 35590497\G

shows the contact's language was updated from en_IN to ta - which is not valid - it should have been ta_IN - this upset thank you mail

Fail mail stopped once I fixed the contact

Event Timeline

This was a similar patch where we caught invalid language codes during import and default to en_US: https://gerrit.wikimedia.org/r/c/wikimedia/fundraising/crm/+/820751

so we should pick the 'best' language for the 2 letter language code if the 2 + 2 variant doesn't exist - ie

for ta_EN - there is no 'Tamil English' so we should choose based on 'ta'. I actually deleted all the variants of 'Tamil' in our DB - which might have been the issue here - but if we ARE choosing between a real one & some fake ones the label & name will be the same for the fake ones - ie what we had was

Namelabel
ta_INTamil
ta_USta_US
ta_QAta_QA

so the 'fake' ones have no real label

We got another TY mail failure on a donation to contact ID 15009262 whose preferred language had been set to just 'en' - the last donation had ENGAGE as the gateway - maybe something funky with an import? I don't see any language logic in offline2civicrm - does a preferred_language column on a csv just get passed directly to the db?

greg triaged this task as Medium priority.Nov 1 2022, 6:15 PM

Change 854004 had a related patch set uploaded (by Damilare Adedoyin; author: Damilare Adedoyin):

[wikimedia/fundraising/crm@master] Return fallback language

https://gerrit.wikimedia.org/r/854004

Noticed the Engage data doesn't come with a language column and the EngageChecksFile doesn't require language. The patch above is just to ensure the imported contact has the default language assigned. Not exactly a solution to this, but might be worth looking at also

Since the last instance of this being recorded as an issue (13 Oct) Wenjun removed 'en' from the option value table - so we are no longer getting new instances of it

select is_deleted, count(*) FROM civicrm_contact WHERE preferred_language = 'en'GROUP BY is_deleted;
+------------+----------+

is_deletedcount(*)

+------------+----------+

110821

+------------+----------+

Have we hit another problem - or was this phab just not kept up to date with issues elsewhere? (Maybe when I get to the bottom of my email I will see :-)

Hi @Eileenmcnaughton, we didn't really hit a new problem. The task was still on CC backlog and so I dug into it. Looks like Engage imports do not have a Language column set in the table, therefore contacts imported from Engage would have their preferred language set to null by default.

select count(*) from civicrm.civicrm_contact where preferred_language is null;
+----------+
| count(*) |
+----------+
|   234771 |
+----------+

I thought to ensure these imported contacts have the default value for the preferred language rather than been set to null. I'm happy to abandon the patch if it's unnecessary.

A better solution probably would have been to map a default locale to each country, however looks like we don't have language per country in the option group.

@Damilare ah that makes sense - I didn't look for NULL - I'll look again

@Damilare @jgleeson OK - now I remember - this is a 'feature' - but I suspect it is a feature we could re-consider.

There is a setting within Civi that allows us to specify what to do when the language is not set. Default Civi behaviour is to set a language. But, before my time, Civi was hacked not to do that & I wound up doing a PR so CiviCRM would support that https://github.com/civicrm/civicrm-core/pull/6728 in order to remove the hack & upgrade us onto the, then modern, 4.6 version of CiviCRM.

image.png (496×868 px, 77 KB)

image.png (442×758 px, 53 KB)

The question is .... WHY ... at the time Adam was the technical lead & he was very strong on respecting what info the user had or hadn't given us. Over time we have moved towards 'display in the UI what is used in Acoustic' - which is where I lean.

I *think* that if we change that setting then we will get the same behaviour (provided in the civicrm core code) as your patch would do in the wmf code - and if not we would at least be able to use that setting instead of the constant or whatever.

The question is whether it is now a feature or a bug - if @Ejegg agrees I suggest we change the setting in the UI in the first instance - note it would also come out of our custom code

image.png (544×1 px, 115 KB)

greg set the point value for this task to 4.
Dwisehaupt removed the point value for this task.Nov 9 2022, 8:41 PM
Dwisehaupt set Final Story Points to 4.