diff --git a/app/controllers/spree/user_sessions_controller.rb b/app/controllers/spree/user_sessions_controller.rb index 82ca577fc3..df42f622e1 100644 --- a/app/controllers/spree/user_sessions_controller.rb +++ b/app/controllers/spree/user_sessions_controller.rb @@ -11,6 +11,7 @@ module Spree ssl_allowed :login_bar before_action :set_checkout_redirect, only: :create + after_action :ensure_valid_locale_persisted, only: :create def create authenticate_spree_user! @@ -48,5 +49,13 @@ module Spree redirect_to(session["spree_user_return_to"] || default) session["spree_user_return_to"] = nil end + + def ensure_valid_locale_persisted + # When creating a new user session we have to wait until after a successful + # login to be able to persist a selected locale on the current user + + UserLocaleSetter.new(spree_current_user, params[:locale], cookies). + ensure_valid_locale_persisted + end end end diff --git a/app/helpers/i18n_helper.rb b/app/helpers/i18n_helper.rb index a04405ac97..8c827e1bba 100644 --- a/app/helpers/i18n_helper.rb +++ b/app/helpers/i18n_helper.rb @@ -1,32 +1,9 @@ module I18nHelper def set_locale - # Save a given locale - if params[:locale] && available_locale?(params[:locale]) - spree_current_user&.update!(locale: params[:locale]) - cookies[:locale] = params[:locale] - end - - # After logging in, check if the user chose a locale before - if spree_current_user && spree_current_user.locale.nil? && cookies[:locale] - spree_current_user.update!(locale: params[:locale]) - end - - I18n.locale = spree_current_user.andand.locale || cookies[:locale] || I18n.default_locale + UserLocaleSetter.new(spree_current_user, params[:locale], cookies).set_locale end def valid_locale(user) - if user.present? && - user.locale.present? && - available_locale?(user.locale) - user.locale - else - I18n.default_locale - end - end - - private - - def available_locale?(locale) - Rails.application.config.i18n.available_locales.include?(locale) + UserLocaleSetter.new(user).valid_current_locale end end diff --git a/app/services/user_locale_setter.rb b/app/services/user_locale_setter.rb new file mode 100644 index 0000000000..cd0fa96631 --- /dev/null +++ b/app/services/user_locale_setter.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +class UserLocaleSetter + def initialize(current_user, params_locale = nil, cookies = {}) + @current_user = current_user + @params_locale = params_locale + @cookies = cookies + end + + def set_locale + save_locale_from_params + + I18n.locale = valid_current_locale + end + + def ensure_valid_locale_persisted + return unless current_user && !available_locale?(current_user.locale) + + current_user.update!(locale: valid_current_locale) + end + + def valid_current_locale + if current_user_locale && available_locale?(current_user_locale) + current_user_locale + elsif cookies[:locale] && available_locale?(cookies[:locale]) + cookies[:locale] + else + I18n.default_locale + end + end + + private + + attr_reader :current_user, :params_locale, :cookies + + def save_locale_from_params + return unless params_locale && available_locale?(params_locale) + + current_user&.update!(locale: params_locale) + cookies[:locale] = params_locale + end + + def available_locale?(locale) + Rails.application.config.i18n.available_locales.include?(locale) + end + + def current_user_locale + current_user&.locale + end +end diff --git a/spec/features/consumer/authentication_spec.rb b/spec/features/consumer/authentication_spec.rb index 201cd45621..c172f40f38 100644 --- a/spec/features/consumer/authentication_spec.rb +++ b/spec/features/consumer/authentication_spec.rb @@ -1,6 +1,7 @@ require 'spec_helper' feature "Authentication", js: true do + include AuthenticationWorkflow include UIComponentHelper include OpenFoodNetwork::EmailHelper @@ -135,5 +136,57 @@ feature "Authentication", js: true do uri = URI.parse(current_url) expect(uri.path + "#" + uri.fragment).to eq('/#/login') end + + describe "with user locales" do + before do + visit root_path + open_login_modal + end + + context "when the user has a valid locale saved" do + before do + user.update!(locale: "es") + end + + it "logs in successfully, applying the saved locale" do + fill_in_and_submit_login_form(user) + expect_logged_in + + expect(page).to have_content I18n.t(:home_shop, locale: :es).upcase + end + end + + context "when the user has an unavailable locale saved" do + before do + user.update!(locale: "xx") + end + + it "logs in successfully and resets the user's locale to the default" do + fill_in_and_submit_login_form(user) + expect_logged_in + + expect(page).to have_content I18n.t(:home_shop, locale: :en).upcase + expect(user.reload.locale).to eq "en" + end + end + + context "when the user has never selected a locale, but one has been selected before login" do + before do + user.update!(locale: nil) + end + + it "logs in successfully and uses the locale from cookies" do + page.driver.browser.manage.add_cookie(name: 'locale', value: 'es') + + fill_in_and_submit_login_form(user) + expect_logged_in + + expect(page).to have_content I18n.t(:home_shop, locale: :es).upcase + expect(user.reload.locale).to eq "es" + + page.driver.browser.manage.delete_cookie('locale') + end + end + end end end diff --git a/spec/services/user_locale_setter_spec.rb b/spec/services/user_locale_setter_spec.rb new file mode 100644 index 0000000000..1c2979a286 --- /dev/null +++ b/spec/services/user_locale_setter_spec.rb @@ -0,0 +1,147 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe UserLocaleSetter do + let(:user) { create(:user) } + let(:default_locale) { I18n.default_locale } + let(:locale_params) { {} } + let(:cookies) { {} } + let(:service) { UserLocaleSetter.new(user, locale_params, cookies) } + + describe "#set_locale" do + describe "persists selected locale from params" do + let(:locale_params) { "es" } + + context "when the user is logged in" do + it "saves selected locale to user.locale and cookies[:locale]" do + service.set_locale + + expect(user.reload.locale).to eq "es" + expect(cookies).to eq({ locale: "es" }) + end + end + + context "when the user is not logged in" do + it "saves selected locale to cookies[:locale]" do + service.set_locale + + expect(cookies).to eq({ locale: "es" }) + end + end + end + + describe "sets the current locale" do + context "when the user is logged in" do + context "and has a valid locale saved" do + before { user.update(locale: "es") } + + it "applies the user's locale" do + service.set_locale + + expect(I18n.locale).to eq :es + end + end + + context "and has an invalid locale saved" do + before { user.update(locale: "xx") } + + it "applies the default locale" do + service.set_locale + + expect(I18n.locale).to eq I18n.default_locale + end + end + + context "and has no locale saved" do + before { user.update(locale: nil) } + + it "applies the locale from cookies if present" do + cookies[:locale] = "es" + service.set_locale + + expect(I18n.locale).to eq :es + end + + it "applies the default locale otherwise " do + service.set_locale + + expect(I18n.locale).to eq I18n.default_locale + end + end + end + + context "when the user is not logged in" do + let(:user) { nil } + + context "with a locale set in cookies" do + it "applies the value from cookies" do + cookies[:locale] = "es" + service.set_locale + + expect(I18n.locale).to eq :es + end + end + + context "with no locale set in cookies" do + it "applies the default locale" do + service.set_locale + + expect(I18n.locale).to eq I18n.default_locale + end + end + end + end + end + + describe "#ensure_valid_locale_persisted" do + context "when a user is present" do + context "and has an unavailable locale saved" do + before { user.update(locale: "xx") } + + context "with no locale set in cookies" do + it "set the user's locale to the default" do + service.ensure_valid_locale_persisted + + expect(user.reload.locale).to eq default_locale.to_s + end + end + + context "with a locale set in cookies" do + let(:cookies) { {locale: "es"} } + + it "set the user's locale to the cookie value" do + service.ensure_valid_locale_persisted + + expect(user.reload.locale).to eq "es" + end + end + end + end + end + + describe "#valid_current_locale" do + let(:service) { UserLocaleSetter.new(user) } + + context "when the user has a locale set" do + it "returns the user's locale" do + user.update(locale: "es") + expect(service.valid_current_locale).to eq "es" + end + end + + context "when the user has no locale set" do + it "returns the default locale" do + expect(service.valid_current_locale).to eq default_locale + end + end + + context "when the given user argument is nil" do + let(:user) { nil } + + it "returns the default locale" do + expect(service.valid_current_locale).to eq default_locale + end + end + end +end diff --git a/spec/support/request/authentication_workflow.rb b/spec/support/request/authentication_workflow.rb index e07327d3e7..a654cf9934 100644 --- a/spec/support/request/authentication_workflow.rb +++ b/spec/support/request/authentication_workflow.rb @@ -68,6 +68,11 @@ module AuthenticationWorkflow fill_in "password", with: user.password click_button "Login" end + + def expect_logged_in + # Ensure page has been reloaded after submitting login form + expect(page).to_not have_selector ".menu #login-link" + end end RSpec.configure do |config|