Problem/Motivation

When the bulk status check iterates through locales and compares them against the smartling_locales_convert_array variable, the casing doesn't match and locales such as "es-LA" are skipped because the value stored in the variable is "es-la".

Proposed resolution

Convert the test locales to lowercase in the comparison.

Remaining tasks

N/A

User interface changes

N/A

API changes

N/A

Data model changes

N/A

CommentFileSizeAuthor
#8 smartling-lowercase-2683595-8-D7.patch1.04 KBbighappyface
#2 smartling-lowercase-2683595-2-D7.patch1.55 KBbighappyface
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bighappyface created an issue. See original summary.

bighappyface’s picture

bighappyface CreditAttribution: bighappyface at Rackspace for Rackspace commented
FileSize
smartling-lowercase-2683595-2-D7.patch1.55 KB
pobster’s picture

pobster CreditAttribution: pobster as a volunteer and for Rackspace commented
Status: Active » Needs review

Looks great to me! +1

Thank you,

Pobster

pobster’s picture

pobster CreditAttribution: pobster as a volunteer and for Rackspace commented
Status: Needs review » Reviewed & tested by the community
Soul88’s picture

Soul88
Primary language Ukrainian
Location Ukraine
CreditAttribution: Soul88 commented
Status: Reviewed & tested by the community » Postponed (maintainer needs more info)

Hey, David.

Could you please give me more info on how did you get the case mismatch?

Because we never lowercase locales from the settings. So the way you get notices is to somehow get the case of locale in code and locale in settings mismatch. And for now I can not see how that could happen.

//one of the possible conclusions from the reasoning above is that if we change the locale case we should also do it before saving to settings.

Soul88’s picture

Soul88
Primary language Ukrainian
Location Ukraine
CreditAttribution: Soul88 commented
Status: Postponed (maintainer needs more info) » Closed (cannot reproduce)

Close for now. Please, feel free to re-open it.

bighappyface’s picture

bighappyface CreditAttribution: bighappyface at Rackspace for Rackspace commented
Assigned: Unassigned » bighappyface
Status: Closed (cannot reproduce) » Needs work

@Soul88 I have re-opened this and have a demonstration of the issue below:

$ bin/drush vget smartling_locales_convert_array
smartling_locales_convert_array: array (
  'es' => 'es-la',
  'pt' => 'pt-br',
  'zh-hans' => 'zh-cn',
  'de' => 'de',
)

Because the locale per Smartling is "es-LA" the convertLocaleSmartlingToDrupal method call returns nothing because it can't find a case-specific match.

How the values were stored as lowercase in that array I do not know, but it certainly wasn't us and I think normalizing strings for the comparison makes sense.

Also, I think this issue is causing problems with the Smartling callback. We are only seeing updates come through for our "de" locale and while we see callbacks in the server access logs for "es-LA" the updates are not yet showing.

I think I will patch convertLocaleSmartlingToDrupal instead of patching the arguments to produce more universal and BC behavior.

bighappyface’s picture

bighappyface CreditAttribution: bighappyface at Rackspace for Rackspace commented
FileSize
smartling-lowercase-2683595-8-D7.patch1.04 KB
bighappyface’s picture

bighappyface CreditAttribution: bighappyface at Rackspace for Rackspace commented
Status: Needs work » Needs review

@Soul88 I have applied the latest path (#8) in production and not only is the bulk status check working, but the callback is working for mixed-case locales as well.

Please review.

Soul88’s picture

Soul88
Primary language Ukrainian
Location Ukraine
CreditAttribution: Soul88 commented
Assigned: bighappyface » Soul88

Will review it shortly

  • Soul88 committed b9577c4 on 7.x-2.x authored by bighappyface 
    Issue #2683595 by bighappyface: Case-insensitive matching in...
Soul88’s picture

Soul88
Primary language Ukrainian
Location Ukraine
CreditAttribution: Soul88 commented

Commited to master, thank you David.

Soul88’s picture

Soul88
Primary language Ukrainian
Location Ukraine
CreditAttribution: Soul88 commented
Status: Needs review » Closed (fixed)