From c4830e3baac32fac64cc15d5727ad803d80c613b Mon Sep 17 00:00:00 2001 From: Pedro Carmona Date: Sun, 3 Sep 2023 23:47:51 +0100 Subject: [PATCH 1/3] Send localized email when creating users via admin interface Based on the current user locale --- .../spree/admin/users_controller.rb | 8 ++- app/helpers/i18n_helper.rb | 6 ++ app/views/spree/admin/users/_form.html.haml | 3 + config/locales/en.yml | 1 + .../spree/users_controller_spec.rb | 3 +- spec/mailers/user_mailer_spec.rb | 32 +++++++-- spec/support/request/authentication_helper.rb | 4 +- spec/system/admin/users_spec.rb | 72 ++++++++++++++++--- 8 files changed, 108 insertions(+), 21 deletions(-) diff --git a/app/controllers/spree/admin/users_controller.rb b/app/controllers/spree/admin/users_controller.rb index 541fd7dbfb..a1b0e3f842 100644 --- a/app/controllers/spree/admin/users_controller.rb +++ b/app/controllers/spree/admin/users_controller.rb @@ -3,6 +3,8 @@ module Spree module Admin class UsersController < ::Admin::ResourceController + helper I18nHelper + rescue_from Spree::User::DestroyWithOrdersError, with: :user_destroy_with_orders_error after_action :sign_in_if_change_own_password, only: :update @@ -127,9 +129,13 @@ module Spree params[:user][:email] != @user.email end + def build_resource + model_class.new(locale: spree_current_user.locale) + end + def user_params ::PermittedAttributes::User.new(params).call( - %i[enterprise_limit show_api_key_view] + %i[enterprise_limit locale show_api_key_view] ) end end diff --git a/app/helpers/i18n_helper.rb b/app/helpers/i18n_helper.rb index 6ca7d9d5b4..c1d21202d4 100644 --- a/app/helpers/i18n_helper.rb +++ b/app/helpers/i18n_helper.rb @@ -1,6 +1,12 @@ # frozen_string_literal: true module I18nHelper + def locales_available + OpenFoodNetwork::I18nConfig.available_locales.map do |locale| + [t('language_name', locale:), locale] + end + end + def set_locale UserLocaleSetter.new(spree_current_user, params[:locale], cookies).set_locale end diff --git a/app/views/spree/admin/users/_form.html.haml b/app/views/spree/admin/users/_form.html.haml index 866f7dffd1..1c43e10fec 100644 --- a/app/views/spree/admin/users/_form.html.haml +++ b/app/views/spree/admin/users/_form.html.haml @@ -12,6 +12,9 @@ = check_box_tag "user[spree_role_ids][]", role.id, @user.spree_roles.include?(role), id: "user_spree_role_#{role.name}" = label_tag role.name = hidden_field_tag "user[spree_role_ids][]", "" + = f.field_container :locale do + = f.label :locale, t(".locale") + = f.select :locale, locales_available, class: "fullwidth" = f.field_container :enterprise_limit do = f.label :enterprise_limit, t(".enterprise_limit") = f.text_field :enterprise_limit, class: "fullwidth" diff --git a/config/locales/en.yml b/config/locales/en.yml index 9020961d32..d3e3e92c35 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -4335,6 +4335,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using enterprise_limit: "Enterprise Limit" confirm_password: "Confirm Password" password: "Password" + locale: "Language" email_confirmation: confirmation_pending: "Email confirmation is pending. We've sent a confirmation email to %{address}." variants: diff --git a/spec/controllers/spree/users_controller_spec.rb b/spec/controllers/spree/users_controller_spec.rb index 9a0e4298fa..8ebffca9c1 100644 --- a/spec/controllers/spree/users_controller_spec.rb +++ b/spec/controllers/spree/users_controller_spec.rb @@ -83,8 +83,9 @@ describe Spree::UsersController, type: :controller do it 'should create a new user' do post :create, params: { user: { email: 'foobar@example.com', password: 'foobar123', - password_confirmation: 'foobar123' } } + password_confirmation: 'foobar123', locale: 'es' } } expect(assigns[:user].new_record?).to be_falsey + expect(assigns[:user].reload.locale).to eq('es') end end end diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index b90b3385fd..2d781fd4ef 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -34,15 +34,33 @@ describe Spree::UserMailer do end describe "#confirmation_instructions" do - it "sends an email" do - token = "random" - email = Spree::UserMailer.confirmation_instructions(user, token) + let(:token) { "random" } + subject(:email) { Spree::UserMailer.confirmation_instructions(user, token) } - expect { + it "sends an email" do + expect { email.deliver_now }.to change { ActionMailer::Base.deliveries.count }.by(1) + end + + context 'when the language is English' do + it 'sends an email with the translated subject' do email.deliver_now - }.to change { - ActionMailer::Base.deliveries.count - }.by(1) + + expect(ActionMailer::Base.deliveries.first.subject).to include( + "Please confirm your OFN account" + ) + end + end + + context 'when the language is Spanish' do + let(:user) { build(:user, locale: 'es') } + + it 'sends an email with the translated subject' do + email.deliver_now + + expect(ActionMailer::Base.deliveries.first.subject).to include( + "Por favor, confirma tu cuenta de OFN" + ) + end end end diff --git a/spec/support/request/authentication_helper.rb b/spec/support/request/authentication_helper.rb index cd76c527c6..ea3e0a39d2 100644 --- a/spec/support/request/authentication_helper.rb +++ b/spec/support/request/authentication_helper.rb @@ -3,8 +3,8 @@ module AuthenticationHelper include Warden::Test::Helpers - def login_as_admin - login_as create(:admin_user) + def login_as_admin(locale: :en) + login_as create(:admin_user, locale:) end def login_to_admin_section diff --git a/spec/system/admin/users_spec.rb b/spec/system/admin/users_spec.rb index 368483e2e7..c44a40c671 100644 --- a/spec/system/admin/users_spec.rb +++ b/spec/system/admin/users_spec.rb @@ -131,21 +131,73 @@ describe "Managing users" do end describe "creating a user" do + let(:locale) { :en } + + before do + login_as_admin(locale:) + end + it "shows no confirmation message to start with" do visit spree.new_admin_user_path expect(page).to have_no_text "Email confirmation is pending" end - it "confirms successful creation" do - visit spree.new_admin_user_path - fill_in "Email", with: "user1@example.org" - fill_in "Password", with: "user1Secret" - fill_in "Confirm Password", with: "user1Secret" - expect do - click_button "Create" - end.to change { Spree::User.count }.by 1 - expect(page).to have_text "Created Successfully" - expect(page).to have_text "Email confirmation is pending" + context "when the current user locale is en" do + let(:locale) { :en } + + it "uses the current user locale for new new user" do + visit spree.new_admin_user_path + + expect(page).to have_select('Language', selected: 'English') + end + + it "confirms successful creation" do + visit spree.new_admin_user_path + fill_in "Email", with: "user1@example.org" + fill_in "Password", with: "user1Secret" + fill_in "Confirm Password", with: "user1Secret" + perform_enqueued_jobs do + expect do + click_button "Create" + end.to change { Spree::User.count }.by 1 + expect(page).to have_text "Created Successfully" + expect(page).to have_text "Email confirmation is pending" + expect(Spree::User.last.locale).to eq "en" + + expect(ActionMailer::Base.deliveries.first.subject).to match( + "Please confirm your OFN account" + ) + end + end + end + + context "when the current user locale is es" do + let(:locale) { :es } + + it "uses the current user locale for new new user" do + visit spree.new_admin_user_path + + expect(page).to have_select('Language', selected: 'Español') + end + + it "confirms successful creation" do + visit spree.new_admin_user_path + fill_in "Email", with: "user1@example.org" + fill_in "Contraseña", with: "user1Secret" + fill_in "Confirmar contraseña", with: "user1Secret" + perform_enqueued_jobs do + expect do + click_button "Crear" + end.to change { Spree::User.count }.by 1 + expect(page).to have_text "Creado con éxito" + expect(page).to have_text "La confirmación por correo electrónico está pendiente" + expect(Spree::User.last.locale).to eq "es" + + expect(ActionMailer::Base.deliveries.first.subject).to match( + "Por favor, confirma tu cuenta de OFN" + ) + end + end end end From 51050036d4648ecb75ffce0f18eb655abbcffb48 Mon Sep 17 00:00:00 2001 From: Pedro Carmona Date: Tue, 5 Sep 2023 00:34:36 +0100 Subject: [PATCH 2/3] Use instance default locale as the default locale for a new user --- .../spree/admin/users_controller.rb | 4 +- app/helpers/i18n_helper.rb | 2 +- app/services/permitted_attributes/user.rb | 2 +- app/views/spree/admin/users/_form.html.haml | 2 +- .../spree/users_controller_spec.rb | 1 - spec/support/request/authentication_helper.rb | 4 +- spec/system/admin/users_spec.rb | 74 +++++-------------- 7 files changed, 26 insertions(+), 63 deletions(-) diff --git a/app/controllers/spree/admin/users_controller.rb b/app/controllers/spree/admin/users_controller.rb index a1b0e3f842..4611095a9c 100644 --- a/app/controllers/spree/admin/users_controller.rb +++ b/app/controllers/spree/admin/users_controller.rb @@ -130,12 +130,12 @@ module Spree end def build_resource - model_class.new(locale: spree_current_user.locale) + model_class.new(locale: I18n.default_locale) end def user_params ::PermittedAttributes::User.new(params).call( - %i[enterprise_limit locale show_api_key_view] + %i[enterprise_limit show_api_key_view] ) end end diff --git a/app/helpers/i18n_helper.rb b/app/helpers/i18n_helper.rb index c1d21202d4..3fca93a061 100644 --- a/app/helpers/i18n_helper.rb +++ b/app/helpers/i18n_helper.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module I18nHelper - def locales_available + def locale_options OpenFoodNetwork::I18nConfig.available_locales.map do |locale| [t('language_name', locale:), locale] end diff --git a/app/services/permitted_attributes/user.rb b/app/services/permitted_attributes/user.rb index 4bb29bbcc4..1e44a92c38 100644 --- a/app/services/permitted_attributes/user.rb +++ b/app/services/permitted_attributes/user.rb @@ -16,7 +16,7 @@ module PermittedAttributes def permitted_attributes [ - :email, :password, :password_confirmation, :disabled, + :email, :locale, :password, :password_confirmation, :disabled, { webhook_endpoints_attributes: [:id, :url] }, ] end diff --git a/app/views/spree/admin/users/_form.html.haml b/app/views/spree/admin/users/_form.html.haml index 1c43e10fec..3eeda73283 100644 --- a/app/views/spree/admin/users/_form.html.haml +++ b/app/views/spree/admin/users/_form.html.haml @@ -14,7 +14,7 @@ = hidden_field_tag "user[spree_role_ids][]", "" = f.field_container :locale do = f.label :locale, t(".locale") - = f.select :locale, locales_available, class: "fullwidth" + = f.select :locale, locale_options, class: "fullwidth" = f.field_container :enterprise_limit do = f.label :enterprise_limit, t(".enterprise_limit") = f.text_field :enterprise_limit, class: "fullwidth" diff --git a/spec/controllers/spree/users_controller_spec.rb b/spec/controllers/spree/users_controller_spec.rb index 8ebffca9c1..3837cf8b71 100644 --- a/spec/controllers/spree/users_controller_spec.rb +++ b/spec/controllers/spree/users_controller_spec.rb @@ -85,7 +85,6 @@ describe Spree::UsersController, type: :controller do params: { user: { email: 'foobar@example.com', password: 'foobar123', password_confirmation: 'foobar123', locale: 'es' } } expect(assigns[:user].new_record?).to be_falsey - expect(assigns[:user].reload.locale).to eq('es') end end end diff --git a/spec/support/request/authentication_helper.rb b/spec/support/request/authentication_helper.rb index ea3e0a39d2..cd76c527c6 100644 --- a/spec/support/request/authentication_helper.rb +++ b/spec/support/request/authentication_helper.rb @@ -3,8 +3,8 @@ module AuthenticationHelper include Warden::Test::Helpers - def login_as_admin(locale: :en) - login_as create(:admin_user, locale:) + def login_as_admin + login_as create(:admin_user) end def login_to_admin_section diff --git a/spec/system/admin/users_spec.rb b/spec/system/admin/users_spec.rb index c44a40c671..6561d06cf6 100644 --- a/spec/system/admin/users_spec.rb +++ b/spec/system/admin/users_spec.rb @@ -131,72 +131,36 @@ describe "Managing users" do end describe "creating a user" do - let(:locale) { :en } - - before do - login_as_admin(locale:) - end - it "shows no confirmation message to start with" do visit spree.new_admin_user_path expect(page).to have_no_text "Email confirmation is pending" end - context "when the current user locale is en" do - let(:locale) { :en } + it "uses the instance default locale for new new user" do + visit spree.new_admin_user_path - it "uses the current user locale for new new user" do - visit spree.new_admin_user_path - - expect(page).to have_select('Language', selected: 'English') - end - - it "confirms successful creation" do - visit spree.new_admin_user_path - fill_in "Email", with: "user1@example.org" - fill_in "Password", with: "user1Secret" - fill_in "Confirm Password", with: "user1Secret" - perform_enqueued_jobs do - expect do - click_button "Create" - end.to change { Spree::User.count }.by 1 - expect(page).to have_text "Created Successfully" - expect(page).to have_text "Email confirmation is pending" - expect(Spree::User.last.locale).to eq "en" - - expect(ActionMailer::Base.deliveries.first.subject).to match( - "Please confirm your OFN account" - ) - end - end + expect(page).to have_select('Language', selected: 'English') end - context "when the current user locale is es" do - let(:locale) { :es } + it "confirms successful creation" do + visit spree.new_admin_user_path + fill_in "Email", with: "user1@example.org" + fill_in "Password", with: "user1Secret" + fill_in "Confirm Password", with: "user1Secret" + select "Español", from: "Language" - it "uses the current user locale for new new user" do - visit spree.new_admin_user_path + perform_enqueued_jobs do + expect do + click_button "Create" + end.to change { Spree::User.count }.by 1 + expect(page).to have_text "Created Successfully" + expect(page).to have_text "Email confirmation is pending" - expect(page).to have_select('Language', selected: 'Español') - end + expect(Spree::User.last.locale).to eq "es" - it "confirms successful creation" do - visit spree.new_admin_user_path - fill_in "Email", with: "user1@example.org" - fill_in "Contraseña", with: "user1Secret" - fill_in "Confirmar contraseña", with: "user1Secret" - perform_enqueued_jobs do - expect do - click_button "Crear" - end.to change { Spree::User.count }.by 1 - expect(page).to have_text "Creado con éxito" - expect(page).to have_text "La confirmación por correo electrónico está pendiente" - expect(Spree::User.last.locale).to eq "es" - - expect(ActionMailer::Base.deliveries.first.subject).to match( - "Por favor, confirma tu cuenta de OFN" - ) - end + expect(ActionMailer::Base.deliveries.first.subject).to match( + "Por favor, confirma tu cuenta de OFN" + ) end end end From 997c8a49ca2dfb54039c1cc442f8404fdaeec850 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 5 Sep 2023 10:51:38 +1000 Subject: [PATCH 3/3] Combine system specs for efficiency --- spec/system/admin/users_spec.rb | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/spec/system/admin/users_spec.rb b/spec/system/admin/users_spec.rb index 6561d06cf6..5d1e7b0163 100644 --- a/spec/system/admin/users_spec.rb +++ b/spec/system/admin/users_spec.rb @@ -131,22 +131,17 @@ describe "Managing users" do end describe "creating a user" do - it "shows no confirmation message to start with" do - visit spree.new_admin_user_path - expect(page).to have_no_text "Email confirmation is pending" - end - - it "uses the instance default locale for new new user" do - visit spree.new_admin_user_path - - expect(page).to have_select('Language', selected: 'English') - end - it "confirms successful creation" do visit spree.new_admin_user_path + + # shows no confirmation message to start with + expect(page).to have_no_text "Email confirmation is pending" + fill_in "Email", with: "user1@example.org" fill_in "Password", with: "user1Secret" fill_in "Confirm Password", with: "user1Secret" + + expect(page).to have_select "Language", selected: "English" select "Español", from: "Language" perform_enqueued_jobs do