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

Fix [sc-25008] - correct and improve legacy language warnings #14401

Merged
merged 1 commit into from
Mar 14, 2024

Conversation

uberbrady
Copy link
Collaborator

The legacy language warning was misfiring when a user's language didn't match the APP_LOCALE from .env.

Additionally, we weren't properly warning when the legacy-language came from Settings or from the user themselves. Both of which should be impossible but still probably not a bad idea to warn on it, anyways

Testing

First, I stashed my changes and prove that the bug exists the way I said -
I will set my user to have a language of 'en-US' and set the APP_LOCALE to 'pt-PT'. I got this message:

Your current APP_LOCALE in your .env is set to "pt-PT" and should be updated to be "en-US" in /Users/uberbrady/Documents/grokability/snipe-it/.env. Translations may display unexpectedly until this is updated.

Then, I reapplied my change and reloaded the dashboard. That message then went away.

Next, I changed the APP_LOCALE to be 'en', and my user's locale to be 'es'. I got this:

APP_LOCALE in .env is set to en and should be updated to be en-US
username admin (1) has a language es and should be updated to be en-US

(Note, we didn't have an es->es-MX fallback in-place, so this behavior is expected)

Finally, I blanked out my user's locale setting, and reloaded the dashboard. The messages I got were:

APP_LOCALE in .env is set to en and should be updated to be en-US
App Settings is set to pt and should be updated to be en-US

(Note, we never had a 'pt' language so it didn't select pt-PT as the fallback)

The legacy language warning was misfiring when a user's language
didn't match the APP_LOCALE from .env.

Additionally, we weren't properly warning when the legacy-language
came from Settings or from the user themselves. Both of which should
be impossible but still probably not a bad idea to warn on it, anyways
@uberbrady uberbrady requested a review from snipe as a code owner March 8, 2024 14:12
Copy link

@uberbrady uberbrady changed the base branch from master to develop March 8, 2024 14:12
Copy link

what-the-diff bot commented Mar 8, 2024

PR Summary

This Pull Request focuses heavily on improving the way our application handles user language preferences. Summarizing the changes:

  • Inclusion of a New Warning Method
    A new warning function, warn_legacy_locale was added to the CheckLocale class. This function only serves to create a warning message if the user's chosen language does not match the legacy version defined in the system.

  • Enhanced Language Setting Warnings
    Warnings about language settings will now be more insightful. They will alert users if the language defined in the '.env' file, their own personal language preference, or the application's preferred language, does not match with the legacy language version.

  • Removal of Redundant Warning
    To avoid redundancy, an existing warning that related to a mismatch in the APP_LOCALE defined in the '.env' file, was removed.

  • Locale Determination
    The locale (i.e., the language) of the application will now be determined using the App::setLocale method, guided by the legacy language version prescribed in the system.

Overall, these changes should make the system more insightful and user-friendly when it comes to handling language preferences.

@snipe snipe merged commit 7bae4c3 into snipe:develop Mar 14, 2024
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants