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

@@ -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

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

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)

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

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

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

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

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