Translate error messages in the activatable and timeoutable hooks #5767
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently I18n.available_locales is called from the tests to ensure that the I18n backend is initialized before the tests run. This, however, is not only unintuitive to initialize the backend, but also unreliable because if the available locales are cached, calling I18n.available_locales won't call further into the backend to initialize it. This is used in combination with I18n.reload! which leaves the backend in uninitialized state but it doesn't clear the global I18n.available_locales, i.e. the available locales are still cached. Consequently, I18n.available_locales won't do what's expected of it - namely initializing the backend by preloading the translations - and tests relying on custom translations may break when the I18n backend initializes itself while looking up a translation key. During this init phase the I18n backend may override translations defined in the tests by translations loaded from the locale YML files. I18n.backend.eager_load! is independent of the cached available locales. If the backend is already initialized, it returns early, otherwise it loads the translations from the YML files.
Pass the used locale to `throw :warden` when the session times out and when an inactive user tries to sign in.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pass down the current locale to the Warden error handler in the activatable and timeoutable hooks.
When used from a Rails application and a timeout occurs or when a user tries to sign in with an unconfirmed account, Devise hands off the error handling to Warden by throwing an exception. Before Warden takes over, the current locale is lost and the error messages are displayed with the default locale. This was addressed in #5567, but he activatable and timeoutable hooks were not in the scope of that PR.
This change closes that gap and ensures that the messages are displayed with the set locale in the aforementioned hooks.
Note that the newly added test cases would pass even without this change, but neither the activatable nor the timeoutable hooks work as expected when Devise is used in the context of a standard Rails app with a specific locale - their error messages would always fall back to the default locale. Passing in the current locale to the
throwmethod, however, fixes this issue in Rails, so this PR just adds the necessary change to the code base.Other notable changes in this PR:
store_translationsby replacing the(locale, translations)argument-pair with a hash.store_translationstest helper in addition to those that are hardcoded in the locale YAML files by disabling the enforcement of available locales in tests. This way temporary translations in any language can be added dynamically without having to add some dummy translation to the YAML files first.I18n.backend.eager_load!instead ofI18n.available_locales. The latter is a non-intuitive way to initialize the backend and it also relies on the cached available locales. WhenI18n.reload!is called to reset the translations, it leaves the backend in uninitialized state but it doesn't clear the cache, meaning that whenI18n.available_localesis called after, the backend may not be initialized. While the backend may have the necessary translations at this point, this may lead to surprising runtime behavior during translations lookup when the tests interact withI18n.Fixes #5765.