From 778b3c3f59fc0f92cbf968ca0ad3b85cefb086fa Mon Sep 17 00:00:00 2001 From: drummer83 Date: Tue, 30 Jan 2024 22:01:16 +0100 Subject: [PATCH] Unify and improve email subjects for all emails Some subjects had a prefix (e.g. [Instance]), some subjects were misleading (e.g. failed payment email had 'Order Confirmation', some can be clearer by adding information like order number, distributor or coordinator --- app/mailers/enterprise_mailer.rb | 5 ++- app/mailers/payment_mailer.rb | 10 +++-- app/mailers/producer_mailer.rb | 6 ++- app/mailers/spree/order_mailer.rb | 32 ++++++++------ app/mailers/spree/shipment_mailer.rb | 7 +-- app/mailers/spree/test_mailer.rb | 2 +- app/mailers/spree/user_mailer.rb | 15 ++++--- app/mailers/subscription_mailer.rb | 64 +++++++++++++++++++--------- config/locales/en.yml | 36 +++++++++------- spec/mailers/report_mailer_spec.rb | 2 +- spec/mailers/shipment_mailer_spec.rb | 6 +-- spec/mailers/user_mailer_spec.rb | 2 +- 12 files changed, 115 insertions(+), 72 deletions(-) diff --git a/app/mailers/enterprise_mailer.rb b/app/mailers/enterprise_mailer.rb index 7704faaa6c..d33baf3be0 100644 --- a/app/mailers/enterprise_mailer.rb +++ b/app/mailers/enterprise_mailer.rb @@ -8,7 +8,7 @@ class EnterpriseMailer < ApplicationMailer def welcome(enterprise) @enterprise = enterprise I18n.with_locale valid_locale(@enterprise.owner) do - subject = t('enterprise_mailer.welcome.subject', + subject = t('.subject', enterprise: @enterprise.name, sitename: Spree::Config[:site_name]) mail(to: enterprise.contact.email, @@ -21,7 +21,8 @@ class EnterpriseMailer < ApplicationMailer @instance = Spree::Config[:site_name] I18n.with_locale valid_locale(@enterprise.owner) do - subject = t('enterprise_mailer.manager_invitation.subject', enterprise: @enterprise.name) + subject = t('.subject', + enterprise: @enterprise.name) mail(to: user.email, subject:) end diff --git a/app/mailers/payment_mailer.rb b/app/mailers/payment_mailer.rb index bf29402cff..7481353b94 100644 --- a/app/mailers/payment_mailer.rb +++ b/app/mailers/payment_mailer.rb @@ -8,9 +8,10 @@ class PaymentMailer < ApplicationMailer @payment = payment @order = @payment.order @hide_ofn_navigation = @payment.order.distributor.hide_ofn_navigation - subject = I18n.t('payment_mailer.authorize_payment.subject', - distributor: @payment.order.distributor.name) I18n.with_locale valid_locale(@payment.order.user) do + subject = t('.subject', + number: @order.number, + distributor: @order.distributor.name) mail(to: payment.order.email, subject:) end end @@ -19,9 +20,10 @@ class PaymentMailer < ApplicationMailer @payment = payment @order = @payment.order shop_owner = @payment.order.distributor.owner - subject = I18n.t('payment_mailer.authorization_required.subject', - order: @payment.order) I18n.with_locale valid_locale(shop_owner) do + subject = t('.subject', + number: @order.number, + distributor: @order.distributor.name) mail(to: shop_owner.email, subject:) end diff --git a/app/mailers/producer_mailer.rb b/app/mailers/producer_mailer.rb index c881a5af65..5d6290794c 100644 --- a/app/mailers/producer_mailer.rb +++ b/app/mailers/producer_mailer.rb @@ -41,8 +41,10 @@ class ProducerMailer < ApplicationMailer end def subject - order_cycle_subject = I18n.t('producer_mailer.order_cycle_report.subject', producer: @producer.name) - "[#{Spree::Config.site_name}] #{order_cycle_subject}" + order_cycle_subject = I18n.t('producer_mailer.order_cycle_report.subject', + coordinator: @coordinator.name, + producer: @producer.name) + "#{order_cycle_subject}" end def orders?(order_cycle, producer) diff --git a/app/mailers/spree/order_mailer.rb b/app/mailers/spree/order_mailer.rb index ee71f84a07..d628f5a18d 100644 --- a/app/mailers/spree/order_mailer.rb +++ b/app/mailers/spree/order_mailer.rb @@ -9,19 +9,24 @@ module Spree helper MailerHelper include I18nHelper - def cancel_email(order_or_order_id, resend = false) + def cancel_email(order_or_order_id) @order = find_order(order_or_order_id) @hide_ofn_navigation = @order.distributor.hide_ofn_navigation I18n.with_locale valid_locale(@order.user) do + subject = t('.subject', + number: @order.number, + distributor: @order.distributor.name) mail(to: @order.email, - subject: mail_subject(t('spree.order_mailer.cancel_email.subject'), resend)) + subject:) end end def cancel_email_for_shop(order) @order = order I18n.with_locale valid_locale(@order.distributor.owner) do - subject = I18n.t('spree.order_mailer.cancel_email_for_shop.subject') + subject = t('.subject', + number: @order.number, + distributor: @order.distributor.name) mail(to: @order.distributor.contact.email, subject:) end @@ -30,18 +35,23 @@ module Spree def confirm_email_for_customer(order_or_order_id, resend = false) @order = find_order(order_or_order_id) @hide_ofn_navigation = @order.distributor.hide_ofn_navigation + resend_prefix = (resend ? "[#{t(:resend).upcase}] " : '') I18n.with_locale valid_locale(@order.user) do - subject = mail_subject(t('spree.order_mailer.confirm_email_for_customer.subject'), resend) + subject = "#{resend_prefix}#{t('.subject', + number: @order.number, + distributor: @order.distributor.name)}" mail(to: @order.email, subject:, reply_to: @order.distributor.contact.email) end end - def confirm_email_for_shop(order_or_order_id, resend = false) + def confirm_email_for_shop(order_or_order_id) @order = find_order(order_or_order_id) I18n.with_locale valid_locale(@order.user) do - subject = mail_subject(t('spree.order_mailer.confirm_email_for_shop.subject'), resend) + subject = t('.subject', + number: @order.number, + distributor: @order.distributor.name) mail(to: @order.distributor.contact.email, subject:) end @@ -65,8 +75,11 @@ module Spree attach_file("invoice-#{@order.number}.pdf", pdf) I18n.with_locale valid_locale(@order.user) do + subject = t('.subject', + number: @order.number, + distributor: @order.distributor.name) mail(to: @order.email, - subject: mail_subject(t(:invoice), false), + subject:, reply_to: @order.distributor.contact.email) end end @@ -78,11 +91,6 @@ module Spree order_or_order_id.respond_to?(:id) ? order_or_order_id : Spree::Order.find(order_or_order_id) end - 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 diff --git a/app/mailers/spree/shipment_mailer.rb b/app/mailers/spree/shipment_mailer.rb index aab69b94d8..8a6f8e38b7 100644 --- a/app/mailers/spree/shipment_mailer.rb +++ b/app/mailers/spree/shipment_mailer.rb @@ -9,15 +9,16 @@ module Spree @order = @shipment.order @hide_ofn_navigation = @shipment.order.distributor.hide_ofn_navigation @delivery = delivery - subject = base_subject + subject = t(base_subject, + number: @order.number, + distributor: @order.distributor.name) mail(to: @shipment.order.email, subject:) end private def base_subject - default_subject = @delivery ? default_i18n_subject : t('.picked_up_subject') - "#{@shipment.order.distributor.name} #{default_subject} ##{@shipment.order.number}" + @delivery ? '.subject' : '.picked_up_subject' end end end diff --git a/app/mailers/spree/test_mailer.rb b/app/mailers/spree/test_mailer.rb index 65ce394ac5..ad5d619cee 100644 --- a/app/mailers/spree/test_mailer.rb +++ b/app/mailers/spree/test_mailer.rb @@ -4,7 +4,7 @@ module Spree class TestMailer < ApplicationMailer def test_email(user) recipient = user.respond_to?(:id) ? user : Spree::User.find(user) - subject = "#{Spree::Config[:site_name]} #{t('spree.test_mailer.test_email.subject')}" + subject = t('.subject', sitename: Spree::Config[:site_name]) mail(to: recipient.email, subject:) end end diff --git a/app/mailers/spree/user_mailer.rb b/app/mailers/spree/user_mailer.rb index 6f2853de2b..8e3d1dbf27 100644 --- a/app/mailers/spree/user_mailer.rb +++ b/app/mailers/spree/user_mailer.rb @@ -8,22 +8,24 @@ module Spree # Overrides `Devise::Mailer.reset_password_instructions` def reset_password_instructions(user, token, _opts = {}) + @instance = Spree::Config[:site_name] @edit_password_reset_url = spree. edit_spree_user_password_url(reset_password_token: token) - subject = "#{Spree::Config[:site_name]} " \ - "#{I18n.t('spree.user_mailer.reset_password_instructions.subject')}" - I18n.with_locale valid_locale(user) do - mail(to: user.email, subject:) + subject = t('.subject', sitename: @instance) + mail(to: user.email, + subject:) end end # This is a OFN specific email, not from Devise::Mailer def signup_confirmation(user) @user = user + @instance = Spree::Config[:site_name] I18n.with_locale valid_locale(@user) do + subject = t('.subject', sitename: @instance) mail(to: user.email, - subject: "#{t(:welcome_to)} #{Spree::Config[:site_name]}") + subject:) end end @@ -33,9 +35,8 @@ module Spree @token = token @instance = Spree::Config[:site_name] @contact = ContentConfig.footer_email - I18n.with_locale valid_locale(@user) do - subject = t('spree.user_mailer.confirmation_instructions.subject') + subject = t('.subject', sitename: @instance) mail(to: confirmation_email_address, subject:) end diff --git a/app/mailers/subscription_mailer.rb b/app/mailers/subscription_mailer.rb index cfc9150ea7..898b1845f8 100644 --- a/app/mailers/subscription_mailer.rb +++ b/app/mailers/subscription_mailer.rb @@ -12,7 +12,14 @@ class SubscriptionMailer < ApplicationMailer @type = 'confirmation' @order = order @hide_ofn_navigation = @order.distributor.hide_ofn_navigation - send_mail(order) + I18n.with_locale valid_locale(@order.user) do + subject = t('.subject', + number: @order.number, + distributor: @order.distributor.name) + mail(to: @order.email, + subject:, + reply_to: @order.distributor.contact.email) + end end def empty_email(order, changes) @@ -20,7 +27,14 @@ class SubscriptionMailer < ApplicationMailer @changes = changes @order = order @hide_ofn_navigation = @order.distributor.hide_ofn_navigation - send_mail(order) + I18n.with_locale valid_locale(@order.user) do + subject = t('.subject', + number: @order.number, + distributor: @order.distributor.name) + mail(to: @order.email, + subject:, + reply_to: @order.distributor.contact.email) + end end def placement_email(order, changes) @@ -28,40 +42,48 @@ class SubscriptionMailer < ApplicationMailer @changes = changes @order = order @hide_ofn_navigation = @order.distributor.hide_ofn_navigation - send_mail(order) + I18n.with_locale valid_locale(@order.user) do + subject = t('.subject', + number: @order.number, + distributor: @order.distributor.name) + mail(to: @order.email, + subject:, + reply_to: @order.distributor.contact.email) + end end def failed_payment_email(order) @order = order @hide_ofn_navigation = @order.distributor.hide_ofn_navigation - send_mail(order) + I18n.with_locale valid_locale(@order.user) do + subject = t('.subject', + number: @order.number, + distributor: @order.distributor.name) + mail(to: @order.email, + subject:, + reply_to: @order.distributor.contact.email) + end end def placement_summary_email(summary) @shop = Enterprise.find(summary.shop_id) @summary = summary - mail(to: @shop.contact.email, - subject: "#{Spree::Config[:site_name]} " \ - "#{t('subscription_mailer.placement_summary_email.subject')}") + I18n.with_locale valid_locale(@shop.owner) do + subject = t('.subject', + distributor: @shop.name) + mail(to: @shop.contact.email, + subject:) + end end def confirmation_summary_email(summary) @shop = Enterprise.find(summary.shop_id) @summary = summary - mail(to: @shop.contact.email, - subject: "#{Spree::Config[:site_name]} " \ - "#{t('subscription_mailer.confirmation_summary_email.subject')}") - end - - private - - def send_mail(order) - I18n.with_locale valid_locale(order.user) do - confirm_email_subject = t('spree.order_mailer.confirm_email_for_customer.subject') - subject = "#{Spree::Config[:site_name]} #{confirm_email_subject} ##{order.number}" - mail(to: order.email, - subject:, - reply_to: order.distributor.contact.email) + I18n.with_locale valid_locale(@shop.owner) do + subject = t('.subject', + distributor: @shop.name) + mail(to: @shop.contact.email, + subject:) end end end diff --git a/config/locales/en.yml b/config/locales/en.yml index e049042a6f..0456bf6eac 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -403,16 +403,16 @@ See the %{link} to find out more about %{sitename}'s features and to start using mistakenly_sent: "Not sure why you have received this email? Please contact %{owner_email} for more information." payment_mailer: authorize_payment: - subject: "Please authorize your payment to %{distributor} on OFN" + subject: "Please authorize your payment for order %{number} at %{distributor}" intro_html: "Your payment of %{amount} to %{distributor} requires additional authentication." instructions: "Please visit the following URL to authorize your payment:" authorization_required: - subject: "A payment requires authorization from the customer" + subject: "Authentication from customer required for payment of order %{number} at %{distributor}" intro_html: "A payment for order %{order_number} at %{distributor} requires additional authorization from the customer." instructions: "The customer has been notified via email and the payment will appear as pending until it is authorized." producer_mailer: order_cycle_report: - subject: "Order cycle report for %{producer}" + subject: "Order cycle report from %{coordinator} for %{producer}" intro_html: "An order cycle report is ready for %{producer}." text_before: "Please find below an update about the order cycle ready for:" order_text: "Here is a summary of the orders for your products:" @@ -421,7 +421,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using text_after: "" report_mailer: report_ready: - subject: "Report ready" + subject: "Report ready for download" intro: "Your report is ready for download." message: "The link below will expire after one week." link_label: "%{name}" @@ -431,21 +431,23 @@ See the %{link} to find out more about %{sitename}'s features and to start using edit_false_html: "You can view details of this order at any time." contact_distributor_html: "If you have any questions you can contact %{distributor} via %{email}." placement_email: + subject: "Subscription order %{number} was placed at %{distributor}" intro_html: "You have a new order with %{distributor}." explainer_html: "This order was automatically created for you." contact_distributor_to_change_order_html: "This order was automatically created for you. You can make changes until orders close on %{orders_close_at} by contacting %{distributor} via %{email}." details_html: "Here are the details of your order for %{distributor}:" changes: "Unfortunately, not all products that you requested were available. The original quantities that you requested appear crossed-out below." placement_summary_email: - subject: "A summary of recently placed subscription orders" + subject: "Summary of recently placed subscription orders at %{distributor}" intro_html: "Below is a summary of the subscription orders that have just been placed for %{shop}." confirmation_email: + subject: "Order confirmation of subscription order %{number} at %{distributor}" payment_success_intro_html: "An automatic payment has been processed for your order from %{distributor}." confirmation_intro_html: "Your order with %{distributor} is now confirmed." explainer_html: "This order was automatically placed for you, and it has now been finalised." details_html: "Here's everything you need to know about your order from %{distributor}:" confirmation_summary_email: - subject: "A summary of recently confirmed subscription orders" + subject: "Summary of recently confirmed subscription orders at %{distributor}" intro_html: "Below is a summary of the subscription orders that have just been finalised for %{shop}." summary_overview: total: "A total of %{count} subscriptions were marked for automatic processing." @@ -474,10 +476,12 @@ See the %{link} to find out more about %{sitename}'s features and to start using title: "Other Failure (%{count} orders)" explainer: "Automatic processing of these orders failed for an unknown reason. This should not occur, please contact us if you are seeing this." empty_email: + subject: "Empty subscription order %{number} at %{distributor}" intro_html: "We tried to place a new order with %{distributor}, but had some problems..." explainer_html: "Unfortunately, none of products that you ordered were available, so no order has been placed. The original quantities that you requested appear crossed-out below." details_html: "Here are the details of the unplaced order for %{distributor}:" failed_payment_email: + subject: "Failed payment for subscription order %{number} at %{distributor}" intro_html: "We tried to process a payment for %{distributor}, but had some problems..." explainer_html: "The payment for your subscription with %{distributor} failed because of a problem with your credit card. %{distributor} has been notified of this failed payment." details_html: "Here are the details of the failure provided by the payment gateway:" @@ -3901,7 +3905,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using spree: order_mailer: cancel_email: - subject: "Cancellation of Order" + subject: "Cancellation of order %{number} at %{distributor}" instructions_html: "Your order with %{distributor} has been CANCELED. Please retain this cancellation information for your records." dont_cancel: "If you have changed your mind or don't wish to cancel this order please contact %{email}" order_summary_canceled_html: "Order Summary #%{number} [CANCELED]" @@ -3910,17 +3914,18 @@ See the %{link} to find out more about %{sitename}'s features and to start using unpaid_order: "Your order was unpaid so no refund has been made" credit_order: "Your order was paid so your account has been credited" # Is this used anywhere? cancel_email_for_shop: - subject: "Cancellation of Order" + subject: "Cancellation of order %{number} at %{distributor}" intro: "A customer has cancelled their order #%{number}." view_cancelled_order: "View cancelled order" confirm_email_for_customer: - subject: "Order Confirmation" + subject: "Order confirmation %{number} at %{distributor}" intro_html: "Thanks for shopping at %{distributor}!" details_html: "Here are your order details from %{distributor}:" confirm_email_for_shop: - subject: "Order Confirmation" + subject: "Incoming order %{number} for %{distributor}" intro_html: "Well done! You have a new order for %{distributor}!" invoice_email: + subject: "Invoice for order %{number} at %{distributor}" intro_html: "Please find attached an invoice for your recent order from %{distributor}." order_summary: item: "Item" @@ -3941,8 +3946,8 @@ See the %{link} to find out more about %{sitename}'s features and to start using special_instructions: "Your notes:" shipment_mailer: shipped_email: - subject: "Shipment Notification" - picked_up_subject: "Pick up Notification" + subject: "Shipment notification of order %{number} at %{distributor}" + picked_up_subject: "Pick up notification of order %{number} at %{distributor}" shipped_intro_html: "Your order from %{distributor} has been shipped." picked_up_intro_html: "Your order from %{distributor} has been picked-up." shipment_summary: "Shipment Summary" @@ -3951,18 +3956,19 @@ See the %{link} to find out more about %{sitename}'s features and to start using thanks: "Thank you for your business." test_mailer: test_email: - subject: "Test Mail" + subject: "Test Mail from %{sitename}" greeting: "Congratulations!" message: "If you have received this email, then your email settings are correct." user_mailer: confirmation_instructions: - subject: "Please confirm your OFN account" + subject: "Please confirm your account on %{sitename}" welcome: "Welcome to %{sitename}!" activate_account: "Before we can activate your new account, we need to confirm your email address." click_link: "Please click the link below to confirm your email and to continue setting up your profile." link_label: "Confirm this email address ยป" notice_unexpected: "You received this message because you signed up on %{sitename}, or were invited to sign up by someone you probably know. If you don't understand why you are receiving this email, please write to %{contact}." signup_confirmation: + subject: "Welcome to %{sitename}" welcome: "Welcome to %{sitename}!" confirmed_email: "Thanks for confirming your email." shop_html: "You can now log in at %{link}." @@ -3971,7 +3977,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using If you are a producer or food enterprise, we are excited to have you as a part of the network." help_html: "We welcome all your questions and feedback; you can use the Send Feedback button on the site or email us at %{email}" reset_password_instructions: - subject: "Reset password instructions" + subject: "Reset password instructions for %{sitename}" request_sent_text: | A request to reset your password has been made. If you did not make this request, simply ignore this email. diff --git a/spec/mailers/report_mailer_spec.rb b/spec/mailers/report_mailer_spec.rb index 404b9e9947..eede8e8d7e 100644 --- a/spec/mailers/report_mailer_spec.rb +++ b/spec/mailers/report_mailer_spec.rb @@ -13,7 +13,7 @@ RSpec.describe ReportMailer do let(:blob) { ReportBlob.create_locally!("customers.csv", "report content") } it "notifies about a report" do - expect(email.subject).to eq "Report ready" + expect(email.subject).to eq "Report ready for download" expect(email.body).to have_content "Your report is ready for download." end diff --git a/spec/mailers/shipment_mailer_spec.rb b/spec/mailers/shipment_mailer_spec.rb index 4544b14ad0..0d09a2bc6e 100644 --- a/spec/mailers/shipment_mailer_spec.rb +++ b/spec/mailers/shipment_mailer_spec.rb @@ -35,9 +35,9 @@ RSpec.describe Spree::ShipmentMailer do }.not_to raise_error end - it "includes the distributor's name in the subject" do + it "includes the order number and distributor's name in the subject" do shipment_email = Spree::ShipmentMailer.shipped_email(shipment, delivery: true) - expect(shipment_email.subject).to include("#{distributor.name} Shipment Notification") + expect(shipment_email.subject).to include("Shipment notification of order #{shipment.order.number} at #{distributor.name}") end it "includes the distributor's name in the body" do @@ -52,6 +52,6 @@ RSpec.describe Spree::ShipmentMailer do it "picked_up email has different subject" do shipment_email = Spree::ShipmentMailer.shipped_email(shipment, delivery: false) - expect(shipment_email.subject).to include("#{distributor.name} Pick up Notification") + expect(shipment_email.subject).to include("Pick up notification of order #{shipment.order.number} at #{distributor.name}") end end diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index 14dbb566f3..157a123664 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -46,7 +46,7 @@ RSpec.describe Spree::UserMailer do email.deliver_now expect(ActionMailer::Base.deliveries.first.subject).to include( - "Please confirm your OFN account" + "Please confirm your account on OFN Demo Site" ) end end