From 693b9bd171aaa409af94e018aaafc8a3dd03fe82 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Sun, 7 Apr 2024 00:17:21 +0500 Subject: [PATCH 1/4] 12332 - Fix rubocop Rails/I18nLocaleAssignment errors - use I18n.with_locale method rather than direct locale assignment --- .rubocop_todo.yml | 10 ------- .../user_registrations_controller_spec.rb | 13 +++++---- spec/helpers/i18n_helper_spec.rb | 7 +++-- spec/models/spree/variant_spec.rb | 28 +++++++++---------- spec/system/admin/order_cycles/list_spec.rb | 6 ++-- 5 files changed, 28 insertions(+), 36 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index f0bc6f67b5..52b702e701 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -576,16 +576,6 @@ Rails/HasManyOrHasOneDependent: - 'app/models/spree/tax_rate.rb' - 'app/models/spree/variant.rb' -# Offense count: 8 -# Configuration parameters: Include. -# Include: spec/**/*.rb, test/**/*.rb -Rails/I18nLocaleAssignment: - Exclude: - - 'spec/controllers/user_registrations_controller_spec.rb' - - 'spec/helpers/i18n_helper_spec.rb' - - 'spec/models/spree/variant_spec.rb' - - 'spec/system/admin/order_cycles/list_spec.rb' - # Offense count: 3 Rails/I18nLocaleTexts: Exclude: diff --git a/spec/controllers/user_registrations_controller_spec.rb b/spec/controllers/user_registrations_controller_spec.rb index 8d91a8d25f..fb948cfa4e 100644 --- a/spec/controllers/user_registrations_controller_spec.rb +++ b/spec/controllers/user_registrations_controller_spec.rb @@ -51,12 +51,13 @@ describe UserRegistrationsController, type: :controller do original_i18n_locale = I18n.locale original_locale_cookie = cookies[:locale] - cookies[:locale] = "pt" - post :create, xhr: true, params: { spree_user: user_params, use_route: :spree } - expect(assigns[:user].locale).to eq("pt") - - I18n.locale = original_i18n_locale - cookies[:locale] = original_locale_cookie + # changes to +I18n.locale+ will only persist within the +with_locale+ block + I18n.with_locale(original_i18n_locale) do + cookies[:locale] = "pt" + post :create, xhr: true, params: { spree_user: user_params, use_route: :spree } + expect(assigns[:user].locale).to eq("pt") + cookies[:locale] = original_locale_cookie + end end end end diff --git a/spec/helpers/i18n_helper_spec.rb b/spec/helpers/i18n_helper_spec.rb index c91b22f6af..546719180e 100644 --- a/spec/helpers/i18n_helper_spec.rb +++ b/spec/helpers/i18n_helper_spec.rb @@ -15,9 +15,10 @@ describe I18nHelper, type: :helper do # have to restore I18n.locale for unit tests that don't call the helper, but # rely on translated strings. around do |example| - locale = I18n.locale - example.run - I18n.locale = locale + original_locale = I18n.locale + I18n.with_locale(original_locale) do + example.run + end end context "as guest" do diff --git a/spec/models/spree/variant_spec.rb b/spec/models/spree/variant_spec.rb index aec627bf71..0de7418b8f 100644 --- a/spec/models/spree/variant_spec.rb +++ b/spec/models/spree/variant_spec.rb @@ -60,14 +60,12 @@ describe Spree::Variant do context "price parsing" do before(:each) do - I18n.locale = I18n.default_locale - I18n.backend.store_translations(:de, - { number: { currency: { format: { delimiter: '.', - separator: ',' } } } }) - end - - after do - I18n.locale = I18n.default_locale + default_locale = I18n.default_locale + I18n.with_locale(default_locale) do + I18n.backend.store_translations(:de, + { number: { currency: { format: { delimiter: '.', + separator: ',' } } } }) + end end context "price=" do @@ -80,17 +78,19 @@ describe Spree::Variant do context "with decimal comma" do it "captures the proper amount for a formatted price" do - I18n.locale = :es - variant.price = '1.599,99' - expect(variant.price).to eq 1599.99 + I18n.with_locale(:es) do + variant.price = '1.599,99' + expect(variant.price).to eq 1599.99 + end end end context "with a numeric price" do it "uses the price as is" do - I18n.locale = :es - variant.price = 1599.99 - expect(variant.price).to eq 1599.99 + I18n.with_locale(:es) do + variant.price = 1599.99 + expect(variant.price).to eq 1599.99 + end end end end diff --git a/spec/system/admin/order_cycles/list_spec.rb b/spec/system/admin/order_cycles/list_spec.rb index bcc977e5f8..87cef27c14 100644 --- a/spec/system/admin/order_cycles/list_spec.rb +++ b/spec/system/admin/order_cycles/list_spec.rb @@ -133,9 +133,9 @@ describe ' } around(:each) do |spec| - I18n.locale = :pt - spec.run - I18n.locale = :en + I18n.with_locale(:pt) do + spec.run + end end context 'using datetimepickers' do From ec61cff3876c1f1d32999f0d947d97895ebc1b2b Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Sun, 7 Apr 2024 03:02:37 +0500 Subject: [PATCH 2/4] 12332 - Fix rubocop Rails/I18nLocaleTexts errors - Add en locales for the hardcodded strings --- .rubocop_todo.yml | 5 ----- app/controllers/admin/stripe_accounts_controller.rb | 6 +++--- config/locales/en.yml | 3 +++ 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 52b702e701..0c09f9ce14 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -576,11 +576,6 @@ Rails/HasManyOrHasOneDependent: - 'app/models/spree/tax_rate.rb' - 'app/models/spree/variant.rb' -# Offense count: 3 -Rails/I18nLocaleTexts: - Exclude: - - 'app/controllers/admin/stripe_accounts_controller.rb' - # Offense count: 22 # Configuration parameters: IgnoreScopes, Include. # Include: app/models/**/*.rb diff --git a/app/controllers/admin/stripe_accounts_controller.rb b/app/controllers/admin/stripe_accounts_controller.rb index b72175ac05..3b094adc5e 100644 --- a/app/controllers/admin/stripe_accounts_controller.rb +++ b/app/controllers/admin/stripe_accounts_controller.rb @@ -16,14 +16,14 @@ module Admin authorize! :destroy, stripe_account if stripe_account.deauthorize_and_destroy - flash[:success] = "Stripe account disconnected." + flash[:success] = I18n.t('stripe.success_code.disconnected') else - flash[:error] = "Failed to disconnect Stripe." + flash[:error] = I18n.t('stripe.error_code.disconnect_failure') end redirect_to main_app.edit_admin_enterprise_path(stripe_account.enterprise) rescue ActiveRecord::RecordNotFound - flash[:error] = "Failed to disconnect Stripe." + flash[:error] = I18n.t('stripe.error_code.disconnect_failure') redirect_to spree.admin_dashboard_path end diff --git a/config/locales/en.yml b/config/locales/en.yml index 3c3c1d1c1d..162f65d273 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -235,6 +235,9 @@ en: transaction_not_allowed: "The card has been declined for an unknown reason." try_again_later: "The card has been declined for an unknown reason." withdrawal_count_limit_exceeded: "The customer has exceeded the balance or credit limit available on their card." + disconnect_failure: "Failed to disconnect Stripe." + success_code: + disconnected: "Stripe account disconnected." activemodel: errors: From b2172ef8d8d7b90b7987606a2560ac1238979f90 Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Tue, 9 Apr 2024 05:19:45 +0500 Subject: [PATCH 3/4] 12332 - Add around block to apply default_locale on specs --- spec/models/spree/variant_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/models/spree/variant_spec.rb b/spec/models/spree/variant_spec.rb index 0de7418b8f..b081c355f8 100644 --- a/spec/models/spree/variant_spec.rb +++ b/spec/models/spree/variant_spec.rb @@ -59,12 +59,13 @@ describe Spree::Variant do end context "price parsing" do - before(:each) do + around(:each) do |spec| default_locale = I18n.default_locale I18n.with_locale(default_locale) do I18n.backend.store_translations(:de, { number: { currency: { format: { delimiter: '.', separator: ',' } } } }) + spec.run end end From c2c7910357ed2ff6562cc997e6941cd50bc71d2e Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 11 Apr 2024 10:05:49 +1000 Subject: [PATCH 4/4] Reset I18n.local for each spec This avoids a locale setting leaking from one spec to another. It also means that we don't have to reset the locale in individual specs. Also: - `cookies` is reset automatically and we don't need to do that. - Removed some unused code (German number format and helper methods). --- spec/base_spec_helper.rb | 5 +++++ .../user_registrations_controller_spec.rb | 13 +++---------- spec/helpers/i18n_helper_spec.rb | 11 ----------- spec/models/spree/variant_spec.rb | 10 ---------- spec/system/admin/order_cycles/list_spec.rb | 9 --------- 5 files changed, 8 insertions(+), 40 deletions(-) diff --git a/spec/base_spec_helper.rb b/spec/base_spec_helper.rb index f835bc883b..1238281f00 100644 --- a/spec/base_spec_helper.rb +++ b/spec/base_spec_helper.rb @@ -96,6 +96,11 @@ RSpec.configure do |config| expectations.syntax = :expect end + # Reset locale for all specs. + config.around(:each) do |example| + I18n.with_locale(:en) { example.run } + end + # Reset all feature toggles to prevent leaking. config.before(:suite) do Flipper.features.each(&:remove) diff --git a/spec/controllers/user_registrations_controller_spec.rb b/spec/controllers/user_registrations_controller_spec.rb index fb948cfa4e..d25b811d69 100644 --- a/spec/controllers/user_registrations_controller_spec.rb +++ b/spec/controllers/user_registrations_controller_spec.rb @@ -48,16 +48,9 @@ describe UserRegistrationsController, type: :controller do end it "sets user.locale from cookie on create" do - original_i18n_locale = I18n.locale - original_locale_cookie = cookies[:locale] - - # changes to +I18n.locale+ will only persist within the +with_locale+ block - I18n.with_locale(original_i18n_locale) do - cookies[:locale] = "pt" - post :create, xhr: true, params: { spree_user: user_params, use_route: :spree } - expect(assigns[:user].locale).to eq("pt") - cookies[:locale] = original_locale_cookie - end + cookies[:locale] = "pt" + post :create, xhr: true, params: { spree_user: user_params, use_route: :spree } + expect(assigns[:user].locale).to eq("pt") end end end diff --git a/spec/helpers/i18n_helper_spec.rb b/spec/helpers/i18n_helper_spec.rb index 546719180e..a4117097d4 100644 --- a/spec/helpers/i18n_helper_spec.rb +++ b/spec/helpers/i18n_helper_spec.rb @@ -10,17 +10,6 @@ describe I18nHelper, type: :helper do allow(helper).to receive(:cookies) { cookies } end - # In the real world, the helper is called in every request and sets - # I18n.locale to the chosen locale or the default. For testing purposes we - # have to restore I18n.locale for unit tests that don't call the helper, but - # rely on translated strings. - around do |example| - original_locale = I18n.locale - I18n.with_locale(original_locale) do - example.run - end - end - context "as guest" do before do allow(helper).to receive(:spree_current_user) { nil } diff --git a/spec/models/spree/variant_spec.rb b/spec/models/spree/variant_spec.rb index b081c355f8..1771e860a1 100644 --- a/spec/models/spree/variant_spec.rb +++ b/spec/models/spree/variant_spec.rb @@ -59,16 +59,6 @@ describe Spree::Variant do end context "price parsing" do - around(:each) do |spec| - default_locale = I18n.default_locale - I18n.with_locale(default_locale) do - I18n.backend.store_translations(:de, - { number: { currency: { format: { delimiter: '.', - separator: ',' } } } }) - spec.run - end - end - context "price=" do context "with decimal point" do it "captures the proper amount for a formatted price" do diff --git a/spec/system/admin/order_cycles/list_spec.rb b/spec/system/admin/order_cycles/list_spec.rb index 87cef27c14..308f95a36f 100644 --- a/spec/system/admin/order_cycles/list_spec.rb +++ b/spec/system/admin/order_cycles/list_spec.rb @@ -194,15 +194,6 @@ describe ' private - def wait_for_edit_form_to_load_order_cycle(order_cycle) - expect(page).to have_field "order_cycle_name", with: order_cycle.name - end - - def select_incoming_variant(supplier, exchange_no, variant) - page.find("table.exchanges tr.supplier-#{supplier.id} td.products").click - check "order_cycle_incoming_exchange_#{exchange_no}_variants_#{variant.id}" - end - def date_warning_msg(nbr = 1) "This order cycle is linked to %d open subscription orders. Changing this date now will not " \ "affect any orders which have already been placed, but should be avoided if possible. " \