From feea499298c68cde233da372d0350db664172430 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 6 Dec 2018 11:36:21 +0000 Subject: [PATCH 1/6] Make all mailers use appropriate locale when sending emails --- app/helpers/i18n_helper.rb | 16 ++++++- app/mailers/enterprise_mailer.rb | 26 ++++++----- app/mailers/producer_mailer.rb | 7 +-- app/mailers/spree/order_mailer_decorator.rb | 49 ++++++++++++--------- app/mailers/spree/user_mailer_decorator.rb | 18 +++++--- app/mailers/subscription_mailer.rb | 13 +++--- 6 files changed, 83 insertions(+), 46 deletions(-) diff --git a/app/helpers/i18n_helper.rb b/app/helpers/i18n_helper.rb index 4c93bdc0bc..6ed767d675 100644 --- a/app/helpers/i18n_helper.rb +++ b/app/helpers/i18n_helper.rb @@ -1,7 +1,7 @@ module I18nHelper def set_locale # Save a given locale - if params[:locale] && Rails.application.config.i18n.available_locales.include?(params[:locale]) + if params[:locale] && available_locale?(params[:locale]) spree_current_user.update_attributes!(locale: params[:locale]) if spree_current_user cookies[:locale] = params[:locale] end @@ -13,4 +13,18 @@ module I18nHelper I18n.locale = spree_current_user.andand.locale || cookies[:locale] || I18n.default_locale end + + def valid_locale(locale) + if available_locale?(locale) + locale + else + I18n.default_locale + end + end + + private + + def available_locale?(locale) + Rails.application.config.i18n.available_locales.include?(locale) + end end diff --git a/app/mailers/enterprise_mailer.rb b/app/mailers/enterprise_mailer.rb index 03d9279c93..f8ddbe970e 100644 --- a/app/mailers/enterprise_mailer.rb +++ b/app/mailers/enterprise_mailer.rb @@ -1,15 +1,18 @@ require 'devise/mailers/helpers' class EnterpriseMailer < Spree::BaseMailer include Devise::Mailers::Helpers + include I18nHelper def welcome(enterprise) @enterprise = enterprise - subject = t('enterprise_mailer.welcome.subject', - enterprise: @enterprise.name, - sitename: Spree::Config[:site_name]) - mail(:to => enterprise.contact.email, - :from => from_address, - :subject => subject) + I18n.with_locale valid_locale(@enterprise.owner.locale) do + subject = t('enterprise_mailer.welcome.subject', + enterprise: @enterprise.name, + sitename: Spree::Config[:site_name]) + mail(:to => enterprise.contact.email, + :from => from_address, + :subject => subject) + end end def manager_invitation(enterprise, user) @@ -17,11 +20,12 @@ class EnterpriseMailer < Spree::BaseMailer @instance = Spree::Config[:site_name] @instance_email = from_address - subject = t('enterprise_mailer.invite_manager.subject', enterprise: @enterprise.name) - - mail(to: user.email, - from: from_address, - subject: subject) + I18n.with_locale valid_locale(@enterprise.owner.locale) do + subject = t('enterprise_mailer.invite_manager.subject', enterprise: @enterprise.name) + mail(to: user.email, + from: from_address, + subject: subject) + end end private diff --git a/app/mailers/producer_mailer.rb b/app/mailers/producer_mailer.rb index ad7569dad2..32338c1379 100644 --- a/app/mailers/producer_mailer.rb +++ b/app/mailers/producer_mailer.rb @@ -1,4 +1,5 @@ class ProducerMailer < Spree::BaseMailer + include I18nHelper def order_cycle_report(producer, order_cycle) @producer = producer @@ -10,9 +11,10 @@ class ProducerMailer < Spree::BaseMailer @total = total_from_line_items(line_items) @tax_total = tax_total_from_line_items(line_items) - subject = "[#{Spree::Config.site_name}] #{I18n.t('producer_mailer.order_cycle.subject', producer: producer.name)}" + I18n.with_locale valid_locale(@producer.owner.locale) do + subject = "[#{Spree::Config.site_name}] #{I18n.t('producer_mailer.order_cycle.subject', producer: producer.name)}" - if has_orders?(order_cycle, producer) + return unless has_orders?(order_cycle, producer) mail( to: @producer.contact.email, from: from_address, @@ -23,7 +25,6 @@ class ProducerMailer < Spree::BaseMailer end end - private def has_orders?(order_cycle, producer) diff --git a/app/mailers/spree/order_mailer_decorator.rb b/app/mailers/spree/order_mailer_decorator.rb index a4b2db8b70..25ea4a51da 100644 --- a/app/mailers/spree/order_mailer_decorator.rb +++ b/app/mailers/spree/order_mailer_decorator.rb @@ -2,41 +2,50 @@ Spree::OrderMailer.class_eval do helper HtmlHelper helper CheckoutHelper helper SpreeCurrencyHelper + include I18nHelper def cancel_email(order, resend = false) @order = find_order(order) - subject = (resend ? "[#{t(:resend).upcase}] " : '') - subject += "#{Spree::Config[:site_name]} #{t('order_mailer.cancel_email.subject')} ##{order.number}" - mail(to: order.email, from: from_address, subject: subject) + I18n.with_locale valid_locale(@order.user.locale) do + subject = (resend ? "[#{t(:resend).upcase}] " : '') + subject += "#{Spree::Config[:site_name]} #{t('order_mailer.cancel_email.subject')} ##{order.number}" + mail(to: order.email, from: from_address, subject: subject) + end end def confirm_email_for_customer(order, resend = false) find_order(order) # Finds an order instance from an id - subject = (resend ? "[#{t(:resend).upcase}] " : '') - subject += "#{Spree::Config[:site_name]} #{t('order_mailer.confirm_email.subject')} ##{@order.number}" - mail(:to => @order.email, - :from => from_address, - :subject => subject, - :reply_to => @order.distributor.contact.email) + I18n.with_locale valid_locale(@order.user.locale) do + subject = (resend ? "[#{t(:resend).upcase}] " : '') + subject += "#{Spree::Config[:site_name]} #{t('order_mailer.confirm_email.subject')} ##{@order.number}" + mail(:to => @order.email, + :from => from_address, + :subject => subject, + :reply_to => @order.distributor.contact.email) + end end def confirm_email_for_shop(order, resend = false) find_order(order) # Finds an order instance from an id - subject = (resend ? "[#{t(:resend).upcase}] " : '') - subject += "#{Spree::Config[:site_name]} #{t('order_mailer.confirm_email.subject')} ##{@order.number}" - mail(:to => @order.distributor.contact.email, - :from => from_address, - :subject => subject) + I18n.with_locale valid_locale(@order.user.locale) do + subject = (resend ? "[#{t(:resend).upcase}] " : '') + subject += "#{Spree::Config[:site_name]} #{t('order_mailer.confirm_email.subject')} ##{@order.number}" + mail(:to => @order.distributor.contact.email, + :from => from_address, + :subject => subject) + end end def invoice_email(order, pdf) find_order(order) # Finds an order instance from an id - attachments["invoice-#{@order.number}.pdf"] = pdf if pdf.present? - subject = "#{Spree::Config[:site_name]} #{t(:invoice)} ##{@order.number}" - mail(:to => @order.email, - :from => from_address, - :subject => subject, - :reply_to => @order.distributor.contact.email) + I18n.with_locale valid_locale(@order.user.locale) do + attachments["invoice-#{@order.number}.pdf"] = pdf if pdf.present? + subject = "#{Spree::Config[:site_name]} #{t(:invoice)} ##{@order.number}" + mail(:to => @order.email, + :from => from_address, + :subject => subject, + :reply_to => @order.distributor.contact.email) + end end def find_order(order) diff --git a/app/mailers/spree/user_mailer_decorator.rb b/app/mailers/spree/user_mailer_decorator.rb index dced31e7fd..7b7cb38122 100644 --- a/app/mailers/spree/user_mailer_decorator.rb +++ b/app/mailers/spree/user_mailer_decorator.rb @@ -1,8 +1,12 @@ Spree::UserMailer.class_eval do + include I18nHelper + def signup_confirmation(user) @user = user - mail(:to => user.email, :from => from_address, - :subject => t(:welcome_to) + Spree::Config[:site_name]) + I18n.with_locale valid_locale(@user.locale) do + mail(:to => user.email, :from => from_address, + :subject => t(:welcome_to) + Spree::Config[:site_name]) + end end # Overriding `Spree::UserMailer.confirmation_instructions` which is @@ -12,10 +16,12 @@ Spree::UserMailer.class_eval do @instance = Spree::Config[:site_name] @contact = ContentConfig.footer_email - subject = t('spree.user_mailer.confirmation_instructions.subject') - mail(to: confirmation_email_address, - from: from_address, - subject: subject) + I18n.with_locale valid_locale(@user.locale) do + subject = t('spree.user_mailer.confirmation_instructions.subject') + mail(to: confirmation_email_address, + from: from_address, + subject: subject) + end end private diff --git a/app/mailers/subscription_mailer.rb b/app/mailers/subscription_mailer.rb index 426d94f847..448783ceda 100644 --- a/app/mailers/subscription_mailer.rb +++ b/app/mailers/subscription_mailer.rb @@ -1,6 +1,7 @@ class SubscriptionMailer < Spree::BaseMailer helper CheckoutHelper helper ShopMailHelper + include I18nHelper def confirmation_email(order) @type = 'confirmation' @@ -46,10 +47,12 @@ class SubscriptionMailer < Spree::BaseMailer private def send_mail(order) - subject = "#{Spree::Config[:site_name]} #{t('order_mailer.confirm_email.subject')} ##{order.number}" - mail(to: order.email, - from: from_address, - subject: subject, - reply_to: order.distributor.contact.email) + I18n.with_locale valid_locale(order.user.locale) do + subject = "#{Spree::Config[:site_name]} #{t('order_mailer.confirm_email.subject')} ##{order.number}" + mail(to: order.email, + from: from_address, + subject: subject, + reply_to: order.distributor.contact.email) + end end end From 0f442a911e1c06c93d4b00cb2a1c7ad7f9c34b7c Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 6 Dec 2018 12:48:48 +0000 Subject: [PATCH 2/6] Improve I18nHelper.valid_locale to handle empty objects --- app/helpers/i18n_helper.rb | 6 +++--- app/mailers/enterprise_mailer.rb | 4 ++-- app/mailers/producer_mailer.rb | 2 +- app/mailers/spree/order_mailer_decorator.rb | 8 ++++---- app/mailers/spree/user_mailer_decorator.rb | 4 ++-- app/mailers/subscription_mailer.rb | 2 +- 6 files changed, 13 insertions(+), 13 deletions(-) diff --git a/app/helpers/i18n_helper.rb b/app/helpers/i18n_helper.rb index 6ed767d675..f22f5b0f54 100644 --- a/app/helpers/i18n_helper.rb +++ b/app/helpers/i18n_helper.rb @@ -14,9 +14,9 @@ module I18nHelper I18n.locale = spree_current_user.andand.locale || cookies[:locale] || I18n.default_locale end - def valid_locale(locale) - if available_locale?(locale) - locale + def valid_locale(object_with_locale) + if object_with_locale.present? && object_with_locale.locale.present? && available_locale?(object_with_locale.locale) + object_with_locale.locale else I18n.default_locale end diff --git a/app/mailers/enterprise_mailer.rb b/app/mailers/enterprise_mailer.rb index f8ddbe970e..36a1b071a9 100644 --- a/app/mailers/enterprise_mailer.rb +++ b/app/mailers/enterprise_mailer.rb @@ -5,7 +5,7 @@ class EnterpriseMailer < Spree::BaseMailer def welcome(enterprise) @enterprise = enterprise - I18n.with_locale valid_locale(@enterprise.owner.locale) do + I18n.with_locale valid_locale(@enterprise.owner) do subject = t('enterprise_mailer.welcome.subject', enterprise: @enterprise.name, sitename: Spree::Config[:site_name]) @@ -20,7 +20,7 @@ class EnterpriseMailer < Spree::BaseMailer @instance = Spree::Config[:site_name] @instance_email = from_address - I18n.with_locale valid_locale(@enterprise.owner.locale) do + I18n.with_locale valid_locale(@enterprise.owner) do subject = t('enterprise_mailer.invite_manager.subject', enterprise: @enterprise.name) mail(to: user.email, from: from_address, diff --git a/app/mailers/producer_mailer.rb b/app/mailers/producer_mailer.rb index 32338c1379..a41ec0e0d1 100644 --- a/app/mailers/producer_mailer.rb +++ b/app/mailers/producer_mailer.rb @@ -11,7 +11,7 @@ class ProducerMailer < Spree::BaseMailer @total = total_from_line_items(line_items) @tax_total = tax_total_from_line_items(line_items) - I18n.with_locale valid_locale(@producer.owner.locale) do + I18n.with_locale valid_locale(@producer.owner) do subject = "[#{Spree::Config.site_name}] #{I18n.t('producer_mailer.order_cycle.subject', producer: producer.name)}" return unless has_orders?(order_cycle, producer) diff --git a/app/mailers/spree/order_mailer_decorator.rb b/app/mailers/spree/order_mailer_decorator.rb index 25ea4a51da..c4bedc932a 100644 --- a/app/mailers/spree/order_mailer_decorator.rb +++ b/app/mailers/spree/order_mailer_decorator.rb @@ -6,7 +6,7 @@ Spree::OrderMailer.class_eval do def cancel_email(order, resend = false) @order = find_order(order) - I18n.with_locale valid_locale(@order.user.locale) do + I18n.with_locale valid_locale(@order.user) do subject = (resend ? "[#{t(:resend).upcase}] " : '') subject += "#{Spree::Config[:site_name]} #{t('order_mailer.cancel_email.subject')} ##{order.number}" mail(to: order.email, from: from_address, subject: subject) @@ -15,7 +15,7 @@ Spree::OrderMailer.class_eval do def confirm_email_for_customer(order, resend = false) find_order(order) # Finds an order instance from an id - I18n.with_locale valid_locale(@order.user.locale) do + I18n.with_locale valid_locale(@order.user) do subject = (resend ? "[#{t(:resend).upcase}] " : '') subject += "#{Spree::Config[:site_name]} #{t('order_mailer.confirm_email.subject')} ##{@order.number}" mail(:to => @order.email, @@ -27,7 +27,7 @@ Spree::OrderMailer.class_eval do def confirm_email_for_shop(order, resend = false) find_order(order) # Finds an order instance from an id - I18n.with_locale valid_locale(@order.user.locale) do + I18n.with_locale valid_locale(@order.user) do subject = (resend ? "[#{t(:resend).upcase}] " : '') subject += "#{Spree::Config[:site_name]} #{t('order_mailer.confirm_email.subject')} ##{@order.number}" mail(:to => @order.distributor.contact.email, @@ -38,7 +38,7 @@ Spree::OrderMailer.class_eval do def invoice_email(order, pdf) find_order(order) # Finds an order instance from an id - I18n.with_locale valid_locale(@order.user.locale) do + I18n.with_locale valid_locale(@order.user) do attachments["invoice-#{@order.number}.pdf"] = pdf if pdf.present? subject = "#{Spree::Config[:site_name]} #{t(:invoice)} ##{@order.number}" mail(:to => @order.email, diff --git a/app/mailers/spree/user_mailer_decorator.rb b/app/mailers/spree/user_mailer_decorator.rb index 7b7cb38122..f9e618e348 100644 --- a/app/mailers/spree/user_mailer_decorator.rb +++ b/app/mailers/spree/user_mailer_decorator.rb @@ -3,7 +3,7 @@ Spree::UserMailer.class_eval do def signup_confirmation(user) @user = user - I18n.with_locale valid_locale(@user.locale) do + I18n.with_locale valid_locale(@user) do mail(:to => user.email, :from => from_address, :subject => t(:welcome_to) + Spree::Config[:site_name]) end @@ -16,7 +16,7 @@ Spree::UserMailer.class_eval do @instance = Spree::Config[:site_name] @contact = ContentConfig.footer_email - I18n.with_locale valid_locale(@user.locale) do + I18n.with_locale valid_locale(@user) do subject = t('spree.user_mailer.confirmation_instructions.subject') mail(to: confirmation_email_address, from: from_address, diff --git a/app/mailers/subscription_mailer.rb b/app/mailers/subscription_mailer.rb index 448783ceda..bd2a1fcb21 100644 --- a/app/mailers/subscription_mailer.rb +++ b/app/mailers/subscription_mailer.rb @@ -47,7 +47,7 @@ class SubscriptionMailer < Spree::BaseMailer private def send_mail(order) - I18n.with_locale valid_locale(order.user.locale) do + I18n.with_locale valid_locale(order.user) do subject = "#{Spree::Config[:site_name]} #{t('order_mailer.confirm_email.subject')} ##{order.number}" mail(to: order.email, from: from_address, From b9b91231ae176d68ecd42815543f3e55a482c5a4 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 6 Dec 2018 13:23:06 +0000 Subject: [PATCH 3/6] Fix rubocop issues in mailers --- app/helpers/i18n_helper.rb | 4 +- app/mailers/producer_mailer.rb | 3 +- app/mailers/spree/order_mailer_decorator.rb | 46 ++++++++++++--------- app/mailers/subscription_mailer.rb | 3 +- 4 files changed, 33 insertions(+), 23 deletions(-) diff --git a/app/helpers/i18n_helper.rb b/app/helpers/i18n_helper.rb index f22f5b0f54..215028e5aa 100644 --- a/app/helpers/i18n_helper.rb +++ b/app/helpers/i18n_helper.rb @@ -15,7 +15,9 @@ module I18nHelper end def valid_locale(object_with_locale) - if object_with_locale.present? && object_with_locale.locale.present? && available_locale?(object_with_locale.locale) + if object_with_locale.present? && + object_with_locale.locale.present? && + available_locale?(object_with_locale.locale) object_with_locale.locale else I18n.default_locale diff --git a/app/mailers/producer_mailer.rb b/app/mailers/producer_mailer.rb index a41ec0e0d1..058d5403ba 100644 --- a/app/mailers/producer_mailer.rb +++ b/app/mailers/producer_mailer.rb @@ -12,7 +12,8 @@ class ProducerMailer < Spree::BaseMailer @tax_total = tax_total_from_line_items(line_items) I18n.with_locale valid_locale(@producer.owner) do - subject = "[#{Spree::Config.site_name}] #{I18n.t('producer_mailer.order_cycle.subject', producer: producer.name)}" + order_cycle_subject = I18n.t('producer_mailer.order_cycle.subject', producer: producer.name) + subject = "[#{Spree::Config.site_name}] #{order_cycle_subject}" return unless has_orders?(order_cycle, producer) mail( diff --git a/app/mailers/spree/order_mailer_decorator.rb b/app/mailers/spree/order_mailer_decorator.rb index c4bedc932a..7d2cce4609 100644 --- a/app/mailers/spree/order_mailer_decorator.rb +++ b/app/mailers/spree/order_mailer_decorator.rb @@ -7,48 +7,54 @@ Spree::OrderMailer.class_eval do def cancel_email(order, resend = false) @order = find_order(order) I18n.with_locale valid_locale(@order.user) do - subject = (resend ? "[#{t(:resend).upcase}] " : '') - subject += "#{Spree::Config[:site_name]} #{t('order_mailer.cancel_email.subject')} ##{order.number}" - mail(to: order.email, from: from_address, subject: subject) + mail(to: order.email, + from: from_address, + subject: mail_subject(t('order_mailer.cancel_email.subject'), resend)) end end def confirm_email_for_customer(order, resend = false) find_order(order) # Finds an order instance from an id I18n.with_locale valid_locale(@order.user) do - subject = (resend ? "[#{t(:resend).upcase}] " : '') - subject += "#{Spree::Config[:site_name]} #{t('order_mailer.confirm_email.subject')} ##{@order.number}" - mail(:to => @order.email, - :from => from_address, - :subject => subject, - :reply_to => @order.distributor.contact.email) + mail(to: @order.email, + from: from_address, + subject: mail_subject(t('order_mailer.confirm_email.subject'), resend), + reply_to: @order.distributor.contact.email) end end def confirm_email_for_shop(order, resend = false) find_order(order) # Finds an order instance from an id I18n.with_locale valid_locale(@order.user) do - subject = (resend ? "[#{t(:resend).upcase}] " : '') - subject += "#{Spree::Config[:site_name]} #{t('order_mailer.confirm_email.subject')} ##{@order.number}" - mail(:to => @order.distributor.contact.email, - :from => from_address, - :subject => subject) + mail(to: @order.distributor.contact.email, + from: from_address, + subject: mail_subject(t('order_mailer.confirm_email.subject'), resend)) end end def invoice_email(order, pdf) find_order(order) # Finds an order instance from an id + attach_file("invoice-#{@order.number}.pdf", pdf) I18n.with_locale valid_locale(@order.user) do - attachments["invoice-#{@order.number}.pdf"] = pdf if pdf.present? - subject = "#{Spree::Config[:site_name]} #{t(:invoice)} ##{@order.number}" - mail(:to => @order.email, - :from => from_address, - :subject => subject, - :reply_to => @order.distributor.contact.email) + mail(to: @order.email, + from: from_address, + subject: mail_subject(t(:invoice), false), + reply_to: @order.distributor.contact.email) end end def find_order(order) @order = order.respond_to?(:id) ? order : Spree::Order.find(order) end + + private + + def mail_subject(base_subject, resend) + resend_prefix = (resend ? "[#{t(:resend).upcase}] " : '') + "#{resend_prefix}#{Spree::Config[:site_name]} #{base_subject} ##{@order.number}" + end + + def attach_file(filename, file) + attachments[filename] = file if file.present? + end end diff --git a/app/mailers/subscription_mailer.rb b/app/mailers/subscription_mailer.rb index bd2a1fcb21..0046155499 100644 --- a/app/mailers/subscription_mailer.rb +++ b/app/mailers/subscription_mailer.rb @@ -48,7 +48,8 @@ class SubscriptionMailer < Spree::BaseMailer def send_mail(order) I18n.with_locale valid_locale(order.user) do - subject = "#{Spree::Config[:site_name]} #{t('order_mailer.confirm_email.subject')} ##{order.number}" + confirm_email_subject = t('order_mailer.confirm_email.subject') + subject = "#{Spree::Config[:site_name]} #{confirm_email_subject} ##{order.number}" mail(to: order.email, from: from_address, subject: subject, From e274442d77a3541de50127cbfb5af4443d73744c Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 6 Dec 2018 15:13:32 +0000 Subject: [PATCH 4/6] Cover i18nhelper.valid_locale and user mailer with tests related to localized emails --- .rubocop_todo.yml | 1 - app/helpers/i18n_helper.rb | 10 +++--- spec/helpers/i18n_helper_spec.rb | 58 +++++++++++++++++++++++++------- spec/mailers/user_mailer_spec.rb | 29 ++++++++++++++-- 4 files changed, 76 insertions(+), 22 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 99c836deb1..e8b203c9d6 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -845,7 +845,6 @@ Layout/SpaceInsideHashLiteralBraces: - 'spec/features/admin/reports_spec.rb' - 'spec/features/consumer/shopping/checkout_spec.rb' - 'spec/helpers/checkout_helper_spec.rb' - - 'spec/helpers/i18n_helper_spec.rb' - 'spec/helpers/order_cycles_helper_spec.rb' - 'spec/lib/open_food_network/enterprise_fee_calculator_spec.rb' - 'spec/lib/open_food_network/feature_toggle_spec.rb' diff --git a/app/helpers/i18n_helper.rb b/app/helpers/i18n_helper.rb index 215028e5aa..3293d42d98 100644 --- a/app/helpers/i18n_helper.rb +++ b/app/helpers/i18n_helper.rb @@ -14,11 +14,11 @@ module I18nHelper I18n.locale = spree_current_user.andand.locale || cookies[:locale] || I18n.default_locale end - def valid_locale(object_with_locale) - if object_with_locale.present? && - object_with_locale.locale.present? && - available_locale?(object_with_locale.locale) - object_with_locale.locale + def valid_locale(user) + if user.present? && + user.locale.present? && + available_locale?(user.locale) + user.locale else I18n.default_locale end diff --git a/spec/helpers/i18n_helper_spec.rb b/spec/helpers/i18n_helper_spec.rb index 62c58e2345..fba8ab882b 100644 --- a/spec/helpers/i18n_helper_spec.rb +++ b/spec/helpers/i18n_helper_spec.rb @@ -24,13 +24,13 @@ describe I18nHelper, type: :helper do end it "sets the chosen locale" do - allow(helper).to receive(:params) { {locale: "es"} } + allow(helper).to receive(:params) { { locale: "es" } } helper.set_locale expect(I18n.locale).to eq :es end it "remembers the chosen locale" do - allow(helper).to receive(:params) { {locale: "es"} } + allow(helper).to receive(:params) { { locale: "es" } } helper.set_locale allow(helper).to receive(:params) { {} } @@ -39,16 +39,16 @@ describe I18nHelper, type: :helper do end it "ignores unavailable locales" do - allow(helper).to receive(:params) { {locale: "xx"} } + allow(helper).to receive(:params) { { locale: "xx" } } helper.set_locale expect(I18n.locale).to eq :en end it "remembers the last chosen locale" do - allow(helper).to receive(:params) { {locale: "en"} } + allow(helper).to receive(:params) { { locale: "en" } } helper.set_locale - allow(helper).to receive(:params) { {locale: "es"} } + allow(helper).to receive(:params) { { locale: "es" } } helper.set_locale allow(helper).to receive(:params) { {} } @@ -57,7 +57,7 @@ describe I18nHelper, type: :helper do end it "remembers the chosen locale after logging in" do - allow(helper).to receive(:params) { {locale: "es"} } + allow(helper).to receive(:params) { { locale: "es" } } helper.set_locale # log in @@ -68,7 +68,7 @@ describe I18nHelper, type: :helper do end it "forgets the chosen locale without cookies" do - allow(helper).to receive(:params) { {locale: "es"} } + allow(helper).to receive(:params) { { locale: "es" } } helper.set_locale # clean up cookies @@ -91,14 +91,14 @@ describe I18nHelper, type: :helper do end it "sets the chosen locale" do - allow(helper).to receive(:params) { {locale: "es"} } + allow(helper).to receive(:params) { { locale: "es" } } helper.set_locale expect(I18n.locale).to eq :es expect(user.locale).to eq "es" end it "remembers the chosen locale" do - allow(helper).to receive(:params) { {locale: "es"} } + allow(helper).to receive(:params) { { locale: "es" } } helper.set_locale allow(helper).to receive(:params) { {} } @@ -107,10 +107,10 @@ describe I18nHelper, type: :helper do end it "remembers the last chosen locale" do - allow(helper).to receive(:params) { {locale: "en"} } + allow(helper).to receive(:params) { { locale: "en" } } helper.set_locale - allow(helper).to receive(:params) { {locale: "es"} } + allow(helper).to receive(:params) { { locale: "es" } } helper.set_locale allow(helper).to receive(:params) { {} } @@ -119,7 +119,7 @@ describe I18nHelper, type: :helper do end it "remembers the chosen locale after logging out" do - allow(helper).to receive(:params) { {locale: "es"} } + allow(helper).to receive(:params) { { locale: "es" } } helper.set_locale # log out @@ -130,7 +130,7 @@ describe I18nHelper, type: :helper do end it "remembers the chosen locale on another computer" do - allow(helper).to receive(:params) { {locale: "es"} } + allow(helper).to receive(:params) { { locale: "es" } } helper.set_locale expect(cookies[:locale]).to eq "es" @@ -142,4 +142,36 @@ describe I18nHelper, type: :helper do expect(I18n.locale).to eq :es end end + + context "#valid_locale" do + around do |example| + original_default_locale = I18n.default_locale + original_available_locales = Rails.application.config.i18n.available_locales + I18n.default_locale = "es" + Rails.application.config.i18n.available_locales = ["es", "pt"] + example.run + I18n.default_locale = original_default_locale + Rails.application.config.i18n.available_locales = original_available_locales + end + + let(:user) { build(:user) } + + it "returns default locale if given user is nil" do + expect(helper.valid_locale(nil)).to eq I18n.default_locale + end + + it "returns default locale if locale of given user is nil" do + expect(helper.valid_locale(user)).to eq I18n.default_locale + end + + it "returns default locale if locale of given user is not available" do + user.locale = "cn" + expect(helper.valid_locale(user)).to eq I18n.default_locale + end + + it "returns the locale of the given user if available" do + user.locale = "pt" + expect(helper.valid_locale(user)).to eq "pt" + end + end end diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index 8082d0f6a6..9b4790e5ad 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -17,9 +17,32 @@ describe Spree::UserMailer do setup_email end - it "sends an email when given a user" do - Spree::UserMailer.signup_confirmation(user).deliver - ActionMailer::Base.deliveries.count.should == 1 + describe '#signup_confirmation' do + it "sends email when given a user" do + Spree::UserMailer.signup_confirmation(user).deliver + expect(ActionMailer::Base.deliveries.count).to eq(1) + end + + describe "user locale" do + around do |example| + original_default_locale = I18n.default_locale + I18n.default_locale = 'pt' + example.run + I18n.default_locale = original_default_locale + end + + it "sends email in user locale when user locale is defined" do + user.locale = 'es' + Spree::UserMailer.signup_confirmation(user).deliver + expect(ActionMailer::Base.deliveries.first.body).to include "Gracias por unirte" + end + + it "sends email in default locale when user locale is not available" do + user.locale = 'cn' + Spree::UserMailer.signup_confirmation(user).deliver + expect(ActionMailer::Base.deliveries.first.body).to include "Obrigada por juntar-se" + end + end end # adapted from https://github.com/spree/spree_auth_devise/blob/70737af/spec/mailers/user_mailer_spec.rb From a9222665b84c19140b3da182ac272b327f68b24f Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Tue, 18 Dec 2018 15:26:49 +0000 Subject: [PATCH 5/6] Refactor user registrations controller, early return to reduce indentation --- .../user_registrations_controller.rb | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/app/controllers/user_registrations_controller.rb b/app/controllers/user_registrations_controller.rb index cc43badbb8..cb7cb61684 100644 --- a/app/controllers/user_registrations_controller.rb +++ b/app/controllers/user_registrations_controller.rb @@ -8,22 +8,22 @@ class UserRegistrationsController < Spree::UserRegistrationsController # POST /resource/sign_up def create @user = build_resource(params[:spree_user]) - if resource.save - session[:spree_user_signup] = true - session[:confirmation_return_url] = params[:return_url] - associate_user + unless resource.save + return render_error(@user.errors) + end - respond_to do |format| - format.html do - set_flash_message(:success, :signed_up_but_unconfirmed) - redirect_to after_sign_in_path_for(@user) - end - format.js do - render json: { email: @user.email } - end + session[:spree_user_signup] = true + session[:confirmation_return_url] = params[:return_url] + associate_user + + respond_to do |format| + format.html do + set_flash_message(:success, :signed_up_but_unconfirmed) + redirect_to after_sign_in_path_for(@user) + end + format.js do + render json: { email: @user.email } end - else - render_error(@user.errors) end rescue StandardError => error OpenFoodNetwork::ErrorLogger.notify(error) From 5e960512aceedd443c6b299cd6037e4da585e537 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Tue, 18 Dec 2018 15:29:50 +0000 Subject: [PATCH 6/6] Set user locale on user registrations #create This is done so that user.locale is used in the first confirmation email. This also stores user.locale in the DB from registration. --- app/controllers/user_registrations_controller.rb | 4 ++++ spec/controllers/user_registrations_controller_spec.rb | 10 ++++++++++ 2 files changed, 14 insertions(+) diff --git a/app/controllers/user_registrations_controller.rb b/app/controllers/user_registrations_controller.rb index cb7cb61684..e8e75dc5ee 100644 --- a/app/controllers/user_registrations_controller.rb +++ b/app/controllers/user_registrations_controller.rb @@ -5,9 +5,13 @@ class UserRegistrationsController < Spree::UserRegistrationsController before_filter :set_checkout_redirect, only: :create + include I18nHelper + before_filter :set_locale + # POST /resource/sign_up def create @user = build_resource(params[:spree_user]) + @user.locale = I18n.locale.to_s unless resource.save return render_error(@user.errors) end diff --git a/spec/controllers/user_registrations_controller_spec.rb b/spec/controllers/user_registrations_controller_spec.rb index 7ed7951c50..22d14b383f 100644 --- a/spec/controllers/user_registrations_controller_spec.rb +++ b/spec/controllers/user_registrations_controller_spec.rb @@ -48,6 +48,16 @@ describe UserRegistrationsController, type: :controller do expect(json).to eq({"email" => "test@test.com"}) expect(controller.spree_current_user).to be_nil end + + it "sets user.locale from cookie on create" do + original_locale_cookie = cookies[:locale] + cookies[:locale] = "pt" + + xhr :post, :create, spree_user: user_params, use_route: :spree + + expect(assigns[:user].locale).to eq("pt") + cookies[:locale] = original_locale_cookie + end end context "when registration fails" do