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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow configuration of available locales #10472

Merged
merged 3 commits into from
Apr 22, 2024

Conversation

mitchellhenke
Copy link
Contributor

馃洜 Summary of changes

As we move towards supporting more languages, we will want to be able to enable locales per-environment so that we can continue working on content without yet displaying to end users.

changelog: Internal, Configuration, Allow configuration of available locales
@mitchellhenke mitchellhenke requested a review from a team April 19, 2024 20:48
lib/identity_config.rb Outdated Show resolved Hide resolved
@aduth
Copy link
Member

aduth commented Apr 22, 2024

I was trying to find any places in code we hard-code locales. While I didn't find any that reference the list, I did find this code, which is something we'll want to plan around as we add more locales. Maybe we can add a test that fails if the set of locales that it switches on is different from available_locales?

def language_code_and_voice_id
case I18n.locale.to_sym
when :en
['en-US', 'Joey']
when :fr
['fr-FR', 'Mathieu']
when :es
['es-US', 'Miguel']
else
['en-US', 'Joey']
end
end

@mitchellhenke
Copy link
Contributor Author

I was trying to find any places in code we hard-code locales. While I didn't find any that reference the list, I did find this code, which is something we'll want to plan around as we add more locales. Maybe we can add a test that fails if the set of locales that it switches on is different from available_locales?

def language_code_and_voice_id
case I18n.locale.to_sym
when :en
['en-US', 'Joey']
when :fr
['fr-FR', 'Mathieu']
when :es
['es-US', 'Miguel']
else
['en-US', 'Joey']
end
end

Thanks for catching that. Refactored to add a test.

Copy link
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 馃憤

@mitchellhenke mitchellhenke merged commit ed5f8dd into main Apr 22, 2024
2 checks passed
@mitchellhenke mitchellhenke deleted the mitchellhenke/configurable-languages branch April 22, 2024 15:12
@aduth
Copy link
Member

aduth commented Apr 25, 2024

I found another place we have hardcoded lists of locales. Can this be updated to reference the configuration? Or have a test similar to what was added for VoiceSender?

EMAIL_REGISTRATION_PATHS = ['/sign_up/enter_email', '/en/sign_up/enter_email',
'/es/sign_up/enter_email', '/fr/sign_up/enter_email'].freeze
SIGN_IN_PATHS = ['/', '/en', '/es', '/fr'].freeze

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants