From 3326366c6e41721b34051cf4011451480b97a481 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Thu, 2 Jul 2020 14:10:56 +0200
Subject: [PATCH 01/20] Add specs for applying stored locales during login

---
 spec/features/consumer/authentication_spec.rb | 35 +++++++++++++++++++
 .../request/authentication_workflow.rb        |  5 +++
 2 files changed, 40 insertions(+)

diff --git a/spec/features/consumer/authentication_spec.rb b/spec/features/consumer/authentication_spec.rb
index 201cd45621..cd4921190a 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,39 @@ 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
+
+        xit "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
+    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|

From 8dfaea629ba2f04be40e46c76c52a6633dc45249 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Thu, 2 Jul 2020 12:54:11 +0200
Subject: [PATCH 02/20] Refactor current_user_locale to a new method

---
 app/helpers/i18n_helper.rb | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/app/helpers/i18n_helper.rb b/app/helpers/i18n_helper.rb
index a04405ac97..c37d036f62 100644
--- a/app/helpers/i18n_helper.rb
+++ b/app/helpers/i18n_helper.rb
@@ -1,17 +1,17 @@
 module I18nHelper
   def set_locale
-    # Save a given locale
+    # Save a given locale from params
     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])
+    if 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
+    I18n.locale = current_user_locale || cookies[:locale] || I18n.default_locale
   end
 
   def valid_locale(user)
@@ -29,4 +29,8 @@ module I18nHelper
   def available_locale?(locale)
     Rails.application.config.i18n.available_locales.include?(locale)
   end
+
+  def current_user_locale
+    spree_current_user.andand.locale
+  end
 end

From aa6f4d4fb98585b5bb68df767eb3591692be90e6 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Thu, 2 Jul 2020 12:56:20 +0200
Subject: [PATCH 03/20] Don't set unavailable locales on the current user

---
 app/helpers/i18n_helper.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/helpers/i18n_helper.rb b/app/helpers/i18n_helper.rb
index c37d036f62..5ce1539f05 100644
--- a/app/helpers/i18n_helper.rb
+++ b/app/helpers/i18n_helper.rb
@@ -7,7 +7,7 @@ module I18nHelper
     end
 
     # After logging in, check if the user chose a locale before
-    if current_user_locale.nil? && cookies[:locale]
+    if current_user_locale.nil? && cookies[:locale] && available_locale?(params[:locale])
       spree_current_user&.update!(locale: params[:locale])
     end
 

From d70d61439ace50712bd8e48abb053f3ba11aa4b8 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Thu, 2 Jul 2020 12:58:29 +0200
Subject: [PATCH 04/20] Always return an available locale

---
 app/helpers/i18n_helper.rb | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/app/helpers/i18n_helper.rb b/app/helpers/i18n_helper.rb
index 5ce1539f05..6d4c3845c1 100644
--- a/app/helpers/i18n_helper.rb
+++ b/app/helpers/i18n_helper.rb
@@ -11,7 +11,7 @@ module I18nHelper
       spree_current_user&.update!(locale: params[:locale])
     end
 
-    I18n.locale = current_user_locale || cookies[:locale] || I18n.default_locale
+    I18n.locale = valid_current_locale
   end
 
   def valid_locale(user)
@@ -33,4 +33,14 @@ module I18nHelper
   def current_user_locale
     spree_current_user.andand.locale
   end
+
+  def valid_current_locale
+    if available_locale?(current_user_locale)
+      current_user_locale
+    elsif available_locale?(cookies[:locale])
+      cookies[:locale]
+    else
+      I18n.default_locale
+    end
+  end
 end

From 91880cdbecc285e87f77aaf1aca0fef8fd7563ef Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Thu, 2 Jul 2020 14:33:41 +0200
Subject: [PATCH 05/20] Make I18nHelper `#available_locale?` method public

---
 app/helpers/i18n_helper.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/helpers/i18n_helper.rb b/app/helpers/i18n_helper.rb
index 6d4c3845c1..b755e85a54 100644
--- a/app/helpers/i18n_helper.rb
+++ b/app/helpers/i18n_helper.rb
@@ -24,12 +24,12 @@ module I18nHelper
     end
   end
 
-  private
-
   def available_locale?(locale)
     Rails.application.config.i18n.available_locales.include?(locale)
   end
 
+  private
+
   def current_user_locale
     spree_current_user.andand.locale
   end

From 4e08d2049faa8fc738b3baef8031c5b4fa7fc950 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Thu, 2 Jul 2020 14:35:02 +0200
Subject: [PATCH 06/20] Ensure a valid locale is persisted during login

This action has to be performed here and not in I18nHelper, as spree_current_user is not initialized yet during the other checks / setting the selected locale value in the app
---
 app/controllers/spree/user_sessions_controller.rb | 8 ++++++++
 spec/features/consumer/authentication_spec.rb     | 2 +-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/app/controllers/spree/user_sessions_controller.rb b/app/controllers/spree/user_sessions_controller.rb
index 82ca577fc3..44fb56a312 100644
--- a/app/controllers/spree/user_sessions_controller.rb
+++ b/app/controllers/spree/user_sessions_controller.rb
@@ -6,11 +6,13 @@ module Spree
     include Spree::Core::ControllerHelpers::Common
     include Spree::Core::ControllerHelpers::Order
     include Spree::Core::ControllerHelpers::SSL
+    include I18nHelper
 
     ssl_required :new, :create, :destroy, :update
     ssl_allowed :login_bar
 
     before_action :set_checkout_redirect, only: :create
+    after_action :ensure_valid_locale, only: :create
 
     def create
       authenticate_spree_user!
@@ -48,5 +50,11 @@ module Spree
       redirect_to(session["spree_user_return_to"] || default)
       session["spree_user_return_to"] = nil
     end
+
+    def ensure_valid_locale
+      return unless spree_current_user && !available_locale?(spree_current_user.locale)
+
+      spree_current_user.update!(locale: I18n.default_locale)
+    end
   end
 end
diff --git a/spec/features/consumer/authentication_spec.rb b/spec/features/consumer/authentication_spec.rb
index cd4921190a..3e676b4c1e 100644
--- a/spec/features/consumer/authentication_spec.rb
+++ b/spec/features/consumer/authentication_spec.rb
@@ -161,7 +161,7 @@ feature "Authentication", js: true do
           user.update!(locale: "xx")
         end
 
-        xit "logs in successfully and resets the user's locale to the default" do
+        it "logs in successfully and resets the user's locale to the default" do
           fill_in_and_submit_login_form(user)
           expect_logged_in
 

From c726340ba304a49c2e6c0130201d944b5b4cff2e Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 3 Jul 2020 11:48:39 +0200
Subject: [PATCH 07/20] Extract #save_locale_from_params comment-method

---
 app/helpers/i18n_helper.rb | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/app/helpers/i18n_helper.rb b/app/helpers/i18n_helper.rb
index b755e85a54..83380e07dc 100644
--- a/app/helpers/i18n_helper.rb
+++ b/app/helpers/i18n_helper.rb
@@ -1,10 +1,6 @@
 module I18nHelper
   def set_locale
-    # Save a given locale from params
-    if params[:locale] && available_locale?(params[:locale])
-      spree_current_user&.update!(locale: params[:locale])
-      cookies[:locale] = params[:locale]
-    end
+    save_locale_from_params
 
     # After logging in, check if the user chose a locale before
     if current_user_locale.nil? && cookies[:locale] && available_locale?(params[:locale])
@@ -30,6 +26,13 @@ module I18nHelper
 
   private
 
+  def save_locale_from_params
+    return unless params[:locale] && available_locale?(params[:locale])
+
+    spree_current_user&.update!(locale: params[:locale])
+    cookies[:locale] = params[:locale]
+  end
+
   def current_user_locale
     spree_current_user.andand.locale
   end

From 7a00a3ba1ed914795dbe119aba17424c335a5082 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 3 Jul 2020 11:52:21 +0200
Subject: [PATCH 08/20] Rename method and add explanatory comment

---
 app/controllers/spree/user_sessions_controller.rb | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/app/controllers/spree/user_sessions_controller.rb b/app/controllers/spree/user_sessions_controller.rb
index 44fb56a312..d48f700581 100644
--- a/app/controllers/spree/user_sessions_controller.rb
+++ b/app/controllers/spree/user_sessions_controller.rb
@@ -12,7 +12,7 @@ module Spree
     ssl_allowed :login_bar
 
     before_action :set_checkout_redirect, only: :create
-    after_action :ensure_valid_locale, only: :create
+    after_action :ensure_valid_locale_persisted, only: :create
 
     def create
       authenticate_spree_user!
@@ -51,7 +51,9 @@ module Spree
       session["spree_user_return_to"] = nil
     end
 
-    def ensure_valid_locale
+    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 locale on the current user
       return unless spree_current_user && !available_locale?(spree_current_user.locale)
 
       spree_current_user.update!(locale: I18n.default_locale)

From 02549d1b0fc00ea53152cecd8802f19c72c3695f Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 3 Jul 2020 12:50:39 +0200
Subject: [PATCH 09/20] Extract all locale-setting logic to a service

---
 .../spree/user_sessions_controller.rb         |  4 +-
 app/helpers/i18n_helper.rb                    | 44 +-----------
 app/services/user_locale_setter.rb            | 67 +++++++++++++++++++
 3 files changed, 70 insertions(+), 45 deletions(-)
 create mode 100644 app/services/user_locale_setter.rb

diff --git a/app/controllers/spree/user_sessions_controller.rb b/app/controllers/spree/user_sessions_controller.rb
index d48f700581..323fff9172 100644
--- a/app/controllers/spree/user_sessions_controller.rb
+++ b/app/controllers/spree/user_sessions_controller.rb
@@ -6,7 +6,6 @@ module Spree
     include Spree::Core::ControllerHelpers::Common
     include Spree::Core::ControllerHelpers::Order
     include Spree::Core::ControllerHelpers::SSL
-    include I18nHelper
 
     ssl_required :new, :create, :destroy, :update
     ssl_allowed :login_bar
@@ -54,9 +53,8 @@ module Spree
     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 locale on the current user
-      return unless spree_current_user && !available_locale?(spree_current_user.locale)
 
-      spree_current_user.update!(locale: I18n.default_locale)
+      UserLocaleSetter.ensure_valid_locale_persisted(spree_current_user)
     end
   end
 end
diff --git a/app/helpers/i18n_helper.rb b/app/helpers/i18n_helper.rb
index 83380e07dc..ce128b39f6 100644
--- a/app/helpers/i18n_helper.rb
+++ b/app/helpers/i18n_helper.rb
@@ -1,49 +1,9 @@
 module I18nHelper
   def set_locale
-    save_locale_from_params
-
-    # After logging in, check if the user chose a locale before
-    if current_user_locale.nil? && cookies[:locale] && available_locale?(params[:locale])
-      spree_current_user&.update!(locale: params[:locale])
-    end
-
-    I18n.locale = valid_current_locale
+    UserLocaleSetter.new(spree_current_user, params[:locale], cookies).call
   end
 
   def valid_locale(user)
-    if user.present? &&
-       user.locale.present? &&
-       available_locale?(user.locale)
-      user.locale
-    else
-      I18n.default_locale
-    end
-  end
-
-  def available_locale?(locale)
-    Rails.application.config.i18n.available_locales.include?(locale)
-  end
-
-  private
-
-  def save_locale_from_params
-    return unless params[:locale] && available_locale?(params[:locale])
-
-    spree_current_user&.update!(locale: params[:locale])
-    cookies[:locale] = params[:locale]
-  end
-
-  def current_user_locale
-    spree_current_user.andand.locale
-  end
-
-  def valid_current_locale
-    if available_locale?(current_user_locale)
-      current_user_locale
-    elsif available_locale?(cookies[:locale])
-      cookies[:locale]
-    else
-      I18n.default_locale
-    end
+    UserLocaleSetter.valid_locale_for_user(user)
   end
 end
diff --git a/app/services/user_locale_setter.rb b/app/services/user_locale_setter.rb
new file mode 100644
index 0000000000..c3dee48bf7
--- /dev/null
+++ b/app/services/user_locale_setter.rb
@@ -0,0 +1,67 @@
+# frozen_string_literal: true
+
+class UserLocaleSetter
+  def initialize(current_user, params_locale, cookies)
+    @current_user = current_user
+    @params_locale = params_locale
+    @cookies = cookies
+  end
+
+  def call
+    save_locale_from_params
+
+    # After logging in, check if the user chose a locale before
+    if current_user_locale.nil? && cookies[:locale] && available_locale?(params_locale)
+      current_user&.update!(locale: params_locale)
+    end
+
+    I18n.locale = valid_current_locale
+  end
+
+  def self.ensure_valid_locale_persisted(user)
+    return unless user && !available_locale?(user.locale)
+
+    user.update!(locale: I18n.default_locale)
+  end
+
+  def self.valid_locale_for_user(user)
+    if user.present? && user.locale.present? && available_locale?(user.locale)
+      user.locale
+    else
+      I18n.default_locale
+    end
+  end
+
+  def self.available_locale?(locale)
+    Rails.application.config.i18n.available_locales.include?(locale)
+  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)
+    self.class.available_locale?(locale)
+  end
+
+  def current_user_locale
+    current_user.andand.locale
+  end
+
+  def valid_current_locale
+    if available_locale?(current_user_locale)
+      current_user_locale
+    elsif available_locale?(cookies[:locale])
+      cookies[:locale]
+    else
+      I18n.default_locale
+    end
+  end
+end

From faa7c0a7c595a81234dec6050554e25ea4b87b64 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 3 Jul 2020 14:02:14 +0200
Subject: [PATCH 10/20] Extract save_cookies_from_locale comment-method

---
 app/services/user_locale_setter.rb | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/app/services/user_locale_setter.rb b/app/services/user_locale_setter.rb
index c3dee48bf7..99fcd57e4e 100644
--- a/app/services/user_locale_setter.rb
+++ b/app/services/user_locale_setter.rb
@@ -9,11 +9,7 @@ class UserLocaleSetter
 
   def call
     save_locale_from_params
-
-    # After logging in, check if the user chose a locale before
-    if current_user_locale.nil? && cookies[:locale] && available_locale?(params_locale)
-      current_user&.update!(locale: params_locale)
-    end
+    save_locale_from_cookies
 
     I18n.locale = valid_current_locale
   end
@@ -47,6 +43,13 @@ class UserLocaleSetter
     cookies[:locale] = params_locale
   end
 
+  def save_locale_from_cookies
+    return unless current_user_locale.nil? && cookies[:locale] &&
+                  available_locale?(params_locale)
+
+    current_user&.update!(locale: params_locale)
+  end
+
   def available_locale?(locale)
     self.class.available_locale?(locale)
   end

From ebffa381c6ead1a79b81c1a4253559323fb61856 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 3 Jul 2020 14:06:05 +0200
Subject: [PATCH 11/20] Update cookies[:locale] logic

---
 app/services/user_locale_setter.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/services/user_locale_setter.rb b/app/services/user_locale_setter.rb
index 99fcd57e4e..3e0e5fdde4 100644
--- a/app/services/user_locale_setter.rb
+++ b/app/services/user_locale_setter.rb
@@ -45,9 +45,9 @@ class UserLocaleSetter
 
   def save_locale_from_cookies
     return unless current_user_locale.nil? && cookies[:locale] &&
-                  available_locale?(params_locale)
+                  available_locale?(cookies[:locale])
 
-    current_user&.update!(locale: params_locale)
+    current_user&.update!(locale: cookies[:locale])
   end
 
   def available_locale?(locale)

From dab0add4923cfef0fa89a37c5b100279503d301b Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 3 Jul 2020 14:20:51 +0200
Subject: [PATCH 12/20] Make conditional more concise

---
 app/services/user_locale_setter.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/services/user_locale_setter.rb b/app/services/user_locale_setter.rb
index 3e0e5fdde4..6c97e268e2 100644
--- a/app/services/user_locale_setter.rb
+++ b/app/services/user_locale_setter.rb
@@ -21,7 +21,7 @@ class UserLocaleSetter
   end
 
   def self.valid_locale_for_user(user)
-    if user.present? && user.locale.present? && available_locale?(user.locale)
+    if user.andand.locale.present? && available_locale?(user.locale)
       user.locale
     else
       I18n.default_locale

From 078726dccae26282e9e7148d2f6348b61308fbc8 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 3 Jul 2020 14:21:45 +0200
Subject: [PATCH 13/20] Add explanatory comment on saving selected locale in
 cookies

---
 app/services/user_locale_setter.rb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/services/user_locale_setter.rb b/app/services/user_locale_setter.rb
index 6c97e268e2..1f36f4ea66 100644
--- a/app/services/user_locale_setter.rb
+++ b/app/services/user_locale_setter.rb
@@ -44,6 +44,8 @@ class UserLocaleSetter
   end
 
   def save_locale_from_cookies
+    # If the user account has a selected locale: we ignore the locale set in cookies,
+    # which is persisted per-device but not per-user.
     return unless current_user_locale.nil? && cookies[:locale] &&
                   available_locale?(cookies[:locale])
 

From ab63d2234ca89275c40c63f393cc7848adbb3fd7 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 3 Jul 2020 14:22:32 +0200
Subject: [PATCH 14/20] Guard against nils in conditions

---
 app/services/user_locale_setter.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/services/user_locale_setter.rb b/app/services/user_locale_setter.rb
index 1f36f4ea66..1a5951df20 100644
--- a/app/services/user_locale_setter.rb
+++ b/app/services/user_locale_setter.rb
@@ -61,9 +61,9 @@ class UserLocaleSetter
   end
 
   def valid_current_locale
-    if available_locale?(current_user_locale)
+    if current_user_locale && available_locale?(current_user_locale)
       current_user_locale
-    elsif available_locale?(cookies[:locale])
+    elsif cookies[:locale] && available_locale?(cookies[:locale])
       cookies[:locale]
     else
       I18n.default_locale

From cc7b5e2df359ac4be63e876ffbeb03084413d991 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 3 Jul 2020 16:28:18 +0200
Subject: [PATCH 15/20] Add pending test for setting locale from cookies during
 login

This test currently fails
---
 spec/features/consumer/authentication_spec.rb | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/spec/features/consumer/authentication_spec.rb b/spec/features/consumer/authentication_spec.rb
index 3e676b4c1e..75f3d519d0 100644
--- a/spec/features/consumer/authentication_spec.rb
+++ b/spec/features/consumer/authentication_spec.rb
@@ -169,6 +169,24 @@ feature "Authentication", js: true do
           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
+
+        xit "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

From 0c2fd4bfd20d5df69116e0a37eff956ff7fbac8d Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 3 Jul 2020 16:32:46 +0200
Subject: [PATCH 16/20] Fix ensure_valid_locale_persisted and change public
 interface of service

---
 app/controllers/spree/user_sessions_controller.rb | 5 +++--
 app/helpers/i18n_helper.rb                        | 2 +-
 app/services/user_locale_setter.rb                | 8 ++++----
 spec/features/consumer/authentication_spec.rb     | 2 +-
 4 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/app/controllers/spree/user_sessions_controller.rb b/app/controllers/spree/user_sessions_controller.rb
index 323fff9172..df42f622e1 100644
--- a/app/controllers/spree/user_sessions_controller.rb
+++ b/app/controllers/spree/user_sessions_controller.rb
@@ -52,9 +52,10 @@ module Spree
 
     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 locale on the current user
+      # login to be able to persist a selected locale on the current user
 
-      UserLocaleSetter.ensure_valid_locale_persisted(spree_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 ce128b39f6..f1142472f1 100644
--- a/app/helpers/i18n_helper.rb
+++ b/app/helpers/i18n_helper.rb
@@ -1,6 +1,6 @@
 module I18nHelper
   def set_locale
-    UserLocaleSetter.new(spree_current_user, params[:locale], cookies).call
+    UserLocaleSetter.new(spree_current_user, params[:locale], cookies).set_locale
   end
 
   def valid_locale(user)
diff --git a/app/services/user_locale_setter.rb b/app/services/user_locale_setter.rb
index 1a5951df20..22ca113a70 100644
--- a/app/services/user_locale_setter.rb
+++ b/app/services/user_locale_setter.rb
@@ -7,17 +7,17 @@ class UserLocaleSetter
     @cookies = cookies
   end
 
-  def call
+  def set_locale
     save_locale_from_params
     save_locale_from_cookies
 
     I18n.locale = valid_current_locale
   end
 
-  def self.ensure_valid_locale_persisted(user)
-    return unless user && !available_locale?(user.locale)
+  def ensure_valid_locale_persisted
+    return unless current_user && !available_locale?(current_user.locale)
 
-    user.update!(locale: I18n.default_locale)
+    current_user.update!(locale: valid_current_locale)
   end
 
   def self.valid_locale_for_user(user)
diff --git a/spec/features/consumer/authentication_spec.rb b/spec/features/consumer/authentication_spec.rb
index 75f3d519d0..c172f40f38 100644
--- a/spec/features/consumer/authentication_spec.rb
+++ b/spec/features/consumer/authentication_spec.rb
@@ -175,7 +175,7 @@ feature "Authentication", js: true do
           user.update!(locale: nil)
         end
 
-        xit "logs in successfully and uses the locale from cookies" do
+        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)

From 05c1f093b229ab31fb44ca8b05bd455e94410709 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 3 Jul 2020 16:44:37 +0200
Subject: [PATCH 17/20] Delete dead code

There's nothing done in this bit of code that isn't already done somewhere else, and more effectively.
---
 app/services/user_locale_setter.rb | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/app/services/user_locale_setter.rb b/app/services/user_locale_setter.rb
index 22ca113a70..c51760ee70 100644
--- a/app/services/user_locale_setter.rb
+++ b/app/services/user_locale_setter.rb
@@ -9,7 +9,6 @@ class UserLocaleSetter
 
   def set_locale
     save_locale_from_params
-    save_locale_from_cookies
 
     I18n.locale = valid_current_locale
   end
@@ -43,15 +42,6 @@ class UserLocaleSetter
     cookies[:locale] = params_locale
   end
 
-  def save_locale_from_cookies
-    # If the user account has a selected locale: we ignore the locale set in cookies,
-    # which is persisted per-device but not per-user.
-    return unless current_user_locale.nil? && cookies[:locale] &&
-                  available_locale?(cookies[:locale])
-
-    current_user&.update!(locale: cookies[:locale])
-  end
-
   def available_locale?(locale)
     self.class.available_locale?(locale)
   end

From e2626a0c3bf478c825be640ac2445dcae2a3ac59 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 3 Jul 2020 18:06:23 +0200
Subject: [PATCH 18/20] Add unit tests for UserLocaleSetter service

---
 spec/services/user_locale_setter_spec.rb | 157 +++++++++++++++++++++++
 1 file changed, 157 insertions(+)
 create mode 100644 spec/services/user_locale_setter_spec.rb

diff --git a/spec/services/user_locale_setter_spec.rb b/spec/services/user_locale_setter_spec.rb
new file mode 100644
index 0000000000..443e6e2f7f
--- /dev/null
+++ b/spec/services/user_locale_setter_spec.rb
@@ -0,0 +1,157 @@
+# 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 "self#valid_locale_for_user" do
+    context "when the user has a locale set" do
+      it "returns the user's locale" do
+        user.update(locale: "es")
+        expect(UserLocaleSetter.valid_locale_for_user(user)).to eq "es"
+      end
+    end
+
+    context "when the user has no locale set" do
+      it "returns the default locale" do
+        expect(UserLocaleSetter.valid_locale_for_user(user)).to eq default_locale
+      end
+    end
+
+    context "when the given user argument is nil" do
+      it "returns the default locale" do
+        expect(UserLocaleSetter.valid_locale_for_user(nil)).to eq default_locale
+      end
+    end
+  end
+
+  describe "self#available_locale?" do
+    it "returns true if locale is available" do
+      expect(UserLocaleSetter.available_locale?("es")).to be true
+    end
+
+    it "returns false if locale is not available" do
+      expect(UserLocaleSetter.available_locale?("xx")).to be false
+    end
+
+    it "returns false if nil is given for locale" do
+      expect(UserLocaleSetter.available_locale?(nil)).to be false
+    end
+  end
+end

From f08a5308771b5ab754827528274cc4746c44e8c5 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Sat, 4 Jul 2020 09:51:00 +0200
Subject: [PATCH 19/20] Make #valid_locale_for_user an instance method

---
 app/helpers/i18n_helper.rb               |  2 +-
 app/services/user_locale_setter.rb       | 16 ++++-----------
 spec/services/user_locale_setter_spec.rb | 26 ++++++++----------------
 3 files changed, 13 insertions(+), 31 deletions(-)

diff --git a/app/helpers/i18n_helper.rb b/app/helpers/i18n_helper.rb
index f1142472f1..f4fd4985ed 100644
--- a/app/helpers/i18n_helper.rb
+++ b/app/helpers/i18n_helper.rb
@@ -4,6 +4,6 @@ module I18nHelper
   end
 
   def valid_locale(user)
-    UserLocaleSetter.valid_locale_for_user(user)
+    UserLocaleSetter.new(user).valid_locale_for_user
   end
 end
diff --git a/app/services/user_locale_setter.rb b/app/services/user_locale_setter.rb
index c51760ee70..bf030af905 100644
--- a/app/services/user_locale_setter.rb
+++ b/app/services/user_locale_setter.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class UserLocaleSetter
-  def initialize(current_user, params_locale, cookies)
+  def initialize(current_user, params_locale = nil, cookies = {})
     @current_user = current_user
     @params_locale = params_locale
     @cookies = cookies
@@ -19,16 +19,8 @@ class UserLocaleSetter
     current_user.update!(locale: valid_current_locale)
   end
 
-  def self.valid_locale_for_user(user)
-    if user.andand.locale.present? && available_locale?(user.locale)
-      user.locale
-    else
-      I18n.default_locale
-    end
-  end
-
-  def self.available_locale?(locale)
-    Rails.application.config.i18n.available_locales.include?(locale)
+  def valid_locale_for_user
+    valid_current_locale
   end
 
   private
@@ -43,7 +35,7 @@ class UserLocaleSetter
   end
 
   def available_locale?(locale)
-    self.class.available_locale?(locale)
+    Rails.application.config.i18n.available_locales.include?(locale)
   end
 
   def current_user_locale
diff --git a/spec/services/user_locale_setter_spec.rb b/spec/services/user_locale_setter_spec.rb
index 443e6e2f7f..46981faec5 100644
--- a/spec/services/user_locale_setter_spec.rb
+++ b/spec/services/user_locale_setter_spec.rb
@@ -120,38 +120,28 @@ describe UserLocaleSetter do
     end
   end
 
-  describe "self#valid_locale_for_user" do
+  describe "#valid_locale_for_user" 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(UserLocaleSetter.valid_locale_for_user(user)).to eq "es"
+        expect(service.valid_locale_for_user).to eq "es"
       end
     end
 
     context "when the user has no locale set" do
       it "returns the default locale" do
-        expect(UserLocaleSetter.valid_locale_for_user(user)).to eq default_locale
+        expect(service.valid_locale_for_user).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(UserLocaleSetter.valid_locale_for_user(nil)).to eq default_locale
+        expect(service.valid_locale_for_user).to eq default_locale
       end
     end
   end
-
-  describe "self#available_locale?" do
-    it "returns true if locale is available" do
-      expect(UserLocaleSetter.available_locale?("es")).to be true
-    end
-
-    it "returns false if locale is not available" do
-      expect(UserLocaleSetter.available_locale?("xx")).to be false
-    end
-
-    it "returns false if nil is given for locale" do
-      expect(UserLocaleSetter.available_locale?(nil)).to be false
-    end
-  end
 end

From 96138e9129cbe76af9fcdb5d31a1b5215d60247a Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Sat, 4 Jul 2020 09:53:36 +0200
Subject: [PATCH 20/20] Refactor UserLocaleSetter public methods

---
 app/helpers/i18n_helper.rb               |  2 +-
 app/services/user_locale_setter.rb       | 22 +++++++++-------------
 spec/services/user_locale_setter_spec.rb |  8 ++++----
 3 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/app/helpers/i18n_helper.rb b/app/helpers/i18n_helper.rb
index f4fd4985ed..8c827e1bba 100644
--- a/app/helpers/i18n_helper.rb
+++ b/app/helpers/i18n_helper.rb
@@ -4,6 +4,6 @@ module I18nHelper
   end
 
   def valid_locale(user)
-    UserLocaleSetter.new(user).valid_locale_for_user
+    UserLocaleSetter.new(user).valid_current_locale
   end
 end
diff --git a/app/services/user_locale_setter.rb b/app/services/user_locale_setter.rb
index bf030af905..cd0fa96631 100644
--- a/app/services/user_locale_setter.rb
+++ b/app/services/user_locale_setter.rb
@@ -19,8 +19,14 @@ class UserLocaleSetter
     current_user.update!(locale: valid_current_locale)
   end
 
-  def valid_locale_for_user
-    valid_current_locale
+  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
@@ -39,16 +45,6 @@ class UserLocaleSetter
   end
 
   def current_user_locale
-    current_user.andand.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
+    current_user&.locale
   end
 end
diff --git a/spec/services/user_locale_setter_spec.rb b/spec/services/user_locale_setter_spec.rb
index 46981faec5..1c2979a286 100644
--- a/spec/services/user_locale_setter_spec.rb
+++ b/spec/services/user_locale_setter_spec.rb
@@ -120,19 +120,19 @@ describe UserLocaleSetter do
     end
   end
 
-  describe "#valid_locale_for_user" do
+  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_locale_for_user).to eq "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_locale_for_user).to eq default_locale
+        expect(service.valid_current_locale).to eq default_locale
       end
     end
 
@@ -140,7 +140,7 @@ describe UserLocaleSetter do
       let(:user) { nil }
 
       it "returns the default locale" do
-        expect(service.valid_locale_for_user).to eq default_locale
+        expect(service.valid_current_locale).to eq default_locale
       end
     end
   end