Updates global #5266
Conversation
There was a problem hiding this comment.
$language = 'en'; and $language = 'en-global'; on ln 544. Are they two different languages?
There was a problem hiding this comment.
Yes they are two different languages. en is just for US, en global is for all countries outside of the US we don't explicitly support
There was a problem hiding this comment.
Is the $country_code value suppose to be included in the user_save($user, ['language' => $language]); ?
Perhaps looking up the user country should always be done based on the user language setting? This seems problematic long run if we have more than one country with the same language but need know the users actual country.
There was a problem hiding this comment.
country code doesnt exist yet but yes it will be added.
and ideally the country code is always specified by the fastly headers and/or this variable we're creating. the language conversion hacks have already been made because we didn't have a country field before.
There was a problem hiding this comment.
ACTUALLY the country code exists already and was unintentionally fixed when I updated the get affiliate code logic
|
@angaither I think its ready for re-review. I commented above re: countries, the logic is already in place |
There was a problem hiding this comment.
new_user_global_attributes?
There was a problem hiding this comment.
|
@deadlybutter what do you mean country code was unintentionally fixed? @mshmsh5000 and @sheyd said country code isn't getting passed correctly on friday. looks like that logic was added a while ago |
|
I just created two new users with diff country codes in the headers, printed them out and the country field in address was correct. I can do it again and screenshot it? and the function being called was updated, not that line, thats why it was unintentionally fixed |
|
@deadlybutter address was set correctly in the user profile? |
|
user profile doesnt display country |
|
so I think the issue #5263 for it passing into NS still stands, but we are saving it correctly to the user profile, correct @deadlybutter? |
|
yes and yes. just verified country is in the DB. just needs to be passed into northstar but thats #5263 id rather get this fixed rather than wait for northstar integration as its blocking our testing later |
|
yip 👍 |
3 participants
What's this PR do?
Updates the logic used to seta users language upon registering. See here for summary of what was discussed in #global #5168 (comment)
All of this was also moved into a new function per @angaither 's suggestion here #5263 (comment)
Also updates dosomething_global_convert_country_to_language to return null rather than 'en'
Where should the reviewer start?
dosomething_user_set_global_attributes();
How should this be manually tested?
Create new users and spoof there country codes, check there languages to make sure they match. Also try making a user without a country code header and make sure it goes to English.
Any background context you want to provide?
Slack discussion from here and down https://dosomething.slack.com/archives/global/p1443451428000871
What are the relevant tickets?
Fixes #5168
Unblocks #5263
Also @deezone this is going to break your logic here for users coming from countries outside US/MX/BR https://github.com/DoSomething/phoenix/blob/cf11f1b791a9668ae73f403b8121dc055156b447/lib/modules/dosomething/dosomething_mbp/dosomething_mbp.module#L235