Switch to around_action and I18n.with_locale

Rails does not recommend using `I18n.locale =` to set the locale for controllers because it can leak in multi-threaded applications across requests. See https://guides.rubyonrails.org/i18n.html#managing-the-locale-across-requests. Instead, `I18n.with_locale` should be used with an `around_action`.

Fixes #635.

CHANGELOG.md

@@ -5,6 +5,7 @@ Unreleased
 * Adds support for adding any number of trial days within `EnsureBilling` by adding the `trial_days` field to `BillingConfiguration`
 * Updated AppBridge to 3.7.8 [#1680](https://github.com/Shopify/shopify_app/pull/1680)
 * Support falling back to 2 letter language code locales [#1711](https://github.com/Shopify/shopify_app/pull/1711)
+* Fix locale leaks across requests [#1711](https://github.com/Shopify/shopify_app/pull/1711)
 
 21.6.0 (July 11, 2023)
 ----------

lib/shopify_app/controller_concerns/localization.rb

@@ -5,12 +5,12 @@ module Localization
     extend ActiveSupport::Concern
 
     included do
-      before_action :set_locale
+      around_action :set_locale
     end
 
     private
 
-    def set_locale
+    def set_locale(&action)
       locale = params[:locale] || I18n.default_locale
 
       # Fallback to the 2 letter language code if the requested locale unavailable
@@ -19,9 +19,9 @@ def set_locale
       end
 
       session[:locale] = locale
-      I18n.locale = session[:locale]
+      I18n.with_locale(session[:locale], &action)
     rescue I18n::InvalidLocale
-      I18n.locale = I18n.default_locale
+      I18n.with_locale(I18n.default_locale, &action)
     end
   end
 end

test/shopify_app/controller_concerns/localization_test.rb

@@ -7,45 +7,38 @@
 class LocalizationController < ActionController::Base
   include ShopifyApp::Localization
 
-  before_action :set_locale
-
   def index
-    head(:ok)
+    render(plain: I18n.locale)
   end
 end
 
 class LocalizationTest < ActionController::TestCase
   tests LocalizationController
 
   setup do
-    @original_default_locale = I18n.default_locale
     I18n.available_locales = [:en, :de, :es, :ja, :fr]
   end
 
-  teardown do
-    I18n.default_locale = @original_default_locale
-  end
-
   test "falls back to I18n.default if locale param is not present" do
     I18n.default_locale = :ja
 
     with_test_routes do
       get :index
-      assert_equal :ja, I18n.locale
+      assert_equal :ja, response.body.to_sym
     end
   end
 
   test "set I18n.locale to passed locale param" do
     with_test_routes do
       get :index, params: { locale: "de" }
-      assert_equal :de, I18n.locale
+      assert_equal :de, response.body.to_sym
     end
   end
 
   test "set I18n.locale to the 2 letter language code if fully-qualified locale param requested is not supported" do
     with_test_routes do
       get :index, params: { locale: "es-ES" }
-      assert_equal :es, I18n.locale
+      assert_equal :es, response.body.to_sym
     end
   end
 
@@ -54,7 +47,7 @@ class LocalizationTest < ActionController::TestCase
 
     with_test_routes do
       get :index, params: { locale: "invalid_locale" }
-      assert_equal :en, I18n.locale
+      assert_equal :en, response.body.to_sym
     end
   end