From c263a6fc2f9a98fe0f345f0fc4e32a8ed10d3dcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sen=C3=A9n=20Rodero=20Rodr=C3=ADguez?= Date: Wed, 8 Jun 2022 12:32:38 +0200 Subject: [PATCH] Configure `Rails/I18nLocaleAssignment` cop to scan all Ruby files This cop scans only the tests files by default, but we prefer to scan all application Ruby files, so when a developer uses the class method `I18n.locale=`, the cop will embrace using the method `I18n.with_locale` instead. By doing this way, the cop will help developers to avoid unexpected translation errors. Quoting the Rails 6 guides: > I18n.locale can leak into subsequent requests served by the same thread/process if it is not consistently set in every controller. For example executing I18n.locale = :es in one POST requests will have effects for all later requests to controllers that don't set the locale, but only in that particular thread/process. For that reason, instead of I18n.locale = you can use I18n.with_locale which does not have this leak issue. Now we enabled the cop for all application Ruby files; we have to remove the assignments at the controller level to set the request locale. As Rails 6 guides suggest [1], we can use the `around_action` controller callback to set each request locale without breaking the rule. This cop will warn CONSUL developers when using `I18n.locale` assignment embracing them to use the `I18n.with_locale`instead. [1] https://guides.rubyonrails.org/i18n.html#managing-the-locale-across-requests --- .rubocop.yml | 2 ++ app/controllers/application_controller.rb | 13 +++++++------ app/controllers/management/base_controller.rb | 6 +++--- app/controllers/subscriptions_controller.rb | 9 +++++---- 4 files changed, 17 insertions(+), 13 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 137ef4cbb..baac68548 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -269,6 +269,8 @@ Rails/HttpStatus: Rails/I18nLocaleAssignment: Enabled: true + Include: + - "**/*.rb" Rails/InverseOf: Enabled: true diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index caee4ec18..0a6e67a98 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -11,7 +11,7 @@ class ApplicationController < ActionController::Base before_action :authenticate_http_basic, if: :http_basic_auth_site? before_action :ensure_signup_complete - before_action :set_locale + around_action :switch_locale before_action :track_email_campaign before_action :set_return_url @@ -40,14 +40,15 @@ class ApplicationController < ActionController::Base end end - def set_locale - I18n.locale = current_locale + def switch_locale(&action) + locale = current_locale - if current_user && current_user.locale != I18n.locale.to_s - current_user.update(locale: I18n.locale) + if current_user && current_user.locale != locale.to_s + current_user.update(locale: locale) end - session[:locale] = I18n.locale + session[:locale] = locale + I18n.with_locale(locale, &action) end def current_locale diff --git a/app/controllers/management/base_controller.rb b/app/controllers/management/base_controller.rb index f9e88ed55..81afa78e4 100644 --- a/app/controllers/management/base_controller.rb +++ b/app/controllers/management/base_controller.rb @@ -4,7 +4,7 @@ class Management::BaseController < ActionController::Base default_form_builder ConsulFormBuilder before_action :verify_manager - before_action :set_locale + around_action :switch_locale helper_method :managed_user helper_method :current_user @@ -38,14 +38,14 @@ class Management::BaseController < ActionController::Base redirect_to management_document_verifications_path, alert: message end - def set_locale + def switch_locale(&action) if params[:locale] && I18n.available_locales.include?(params[:locale].to_sym) session[:locale] = params[:locale] end session[:locale] ||= I18n.default_locale - I18n.locale = session[:locale] + I18n.with_locale(session[:locale], &action) end def current_budget diff --git a/app/controllers/subscriptions_controller.rb b/app/controllers/subscriptions_controller.rb index 580bd0e34..163b913b8 100644 --- a/app/controllers/subscriptions_controller.rb +++ b/app/controllers/subscriptions_controller.rb @@ -1,5 +1,6 @@ class SubscriptionsController < ApplicationController - before_action :set_user, :set_user_locale + before_action :set_user + around_action :set_user_locale skip_authorization_check def edit @@ -29,10 +30,10 @@ class SubscriptionsController < ApplicationController [:email_on_comment, :email_on_comment_reply, :email_on_direct_message, :email_digest, :newsletter] end - def set_user_locale + def set_user_locale(&action) if params[:locale].blank? - I18n.locale = I18n.available_locales.find { |locale| locale == @user.locale&.to_sym } - session[:locale] = I18n.locale + session[:locale] = I18n.available_locales.find { |locale| locale == @user.locale&.to_sym } end + I18n.with_locale(session[:locale], &action) end end