Merge pull request #2624 from kristinalim/fix-no_order_link_in_subscription_emails_unless_user

Subscription emails for customer should not link to order page if customer has no user account

app/helpers/shop_mail_helper.rb

@@ -0,0 +1,8 @@
+module ShopMailHelper
+  # Long datetime format string used in emails to customers
+  #
+  # Example: "Fri Aug 31 @ 11:00PM"
+  def mail_long_datetime_format
+    "%a %b %d @ %l:%M%p"
+  end
+end

app/mailers/subscription_mailer.rb

@@ -1,5 +1,6 @@
 class SubscriptionMailer < Spree::BaseMailer
   helper CheckoutHelper
+  helper ShopMailHelper
 
   def confirmation_email(order)
     @type = 'confirmation'

app/views/subscription_mailer/confirmation_email.html.haml

@@ -3,9 +3,10 @@
 
 %p.callout
   = t("email_so_confirmation_explainer_html")
-  = t("email_so_edit_false_html",
-    orders_close_at: l(@order.order_cycle.orders_close_at, format: "%a %b %d @ %l:%M%p"),
-    order_url: spree.order_url(@order))
+  - if @order.user.present?
+    = t("email_so_edit_false_html",
+      orders_close_at: l(@order.order_cycle.orders_close_at, format: mail_long_datetime_format),
+      order_url: spree.order_url(@order))
   = t("email_so_contact_distributor_html", distributor: @order.distributor.name, email: @order.distributor.contact.email)
 
 %p  

app/views/subscription_mailer/failed_payment_email.html.haml

@@ -2,9 +2,10 @@
 
 %p.callout
   = t("email_so_failed_payment_explainer_html", distributor: @order.distributor.name)
-  = t("email_so_edit_false_html",
-    orders_close_at: l(@order.order_cycle.orders_close_at, format: "%a %b %d @ %l:%M%p"),
-    order_url: spree.order_url(@order))
+  - if @order.user.present?
+    = t("email_so_edit_false_html",
+      orders_close_at: l(@order.order_cycle.orders_close_at, format: mail_long_datetime_format),
+      order_url: spree.order_url(@order))
   = t("email_so_contact_distributor_html", distributor: @order.distributor.name, email: @order.distributor.contact.email)
 
 - if @order.errors.any?

app/views/subscription_mailer/placement_email.html.haml

@@ -3,11 +3,18 @@
 
 %p.callout
   = t("email_so_placement_explainer_html")
-  - allow_changes = !!@order.distributor.allow_order_changes?
-  = t("email_so_edit_#{allow_changes}_html",
-    orders_close_at: l(@order.order_cycle.orders_close_at, format: "%a %b %d @ %l:%M%p"),
-    order_url: spree.order_url(@order))
-  = t("email_so_contact_distributor_html", distributor: @order.distributor.name, email: @order.distributor.contact.email)
+
+  - if @order.user.present?
+    - allow_changes = !!@order.distributor.allow_order_changes?
+    = t("email_so_edit_#{allow_changes}_html",
+      orders_close_at: l(@order.order_cycle.orders_close_at, format: mail_long_datetime_format),
+      order_url: spree.order_url(@order))
+    = t("email_so_contact_distributor_html", distributor: @order.distributor.name, email: @order.distributor.contact.email)
+  - else
+    = t("email_so_contact_distributor_to_change_order_html",
+      orders_close_at: l(@order.order_cycle.orders_close_at, format: mail_long_datetime_format),
+      distributor: @order.distributor.name,
+      email: @order.distributor.contact.email)
 
 %p  
 %h4

config/locales/en.yml

@@ -1459,6 +1459,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using
   email_so_edit_true_html: "You can <a href='%{order_url}'>make changes</a> until orders close on %{orders_close_at}."
   email_so_edit_false_html: "You can <a href='%{order_url}'>view details of this order</a> at any time."
   email_so_contact_distributor_html: "If you have any questions you can contact <strong>%{distributor}</strong> via %{email}."
+  email_so_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 <strong>%{distributor}</strong> via %{email}."
   email_so_confirmation_intro_html: "Your order with <strong>%{distributor}</strong> is now confirmed"
   email_so_confirmation_explainer_html: "This order was automatically placed for you, and it has now been finalised."
   email_so_confirmation_details_html: "Here's everything you need to know about your order from <strong>%{distributor}</strong>:"

spec/mailers/subscription_mailer_spec.rb

@@ -6,7 +6,9 @@ describe SubscriptionMailer do
   let!(:mail_method) { create(:mail_method, preferred_mails_from: 'spree@example.com') }
 
   describe "order placement" do
-    let(:subscription) { create(:subscription, with_items: true) }
+    let(:shop) { create(:enterprise) }
+    let(:customer) { create(:customer, enterprise: shop) }
+    let(:subscription) { create(:subscription, shop: shop, customer: customer, with_items: true) }
     let(:proxy_order) { create(:proxy_order, subscription: subscription) }
     let!(:order) { proxy_order.initialise_order! }
 
@@ -24,7 +26,6 @@ describe SubscriptionMailer do
         body = SubscriptionMailer.deliveries.last.body.encoded
         expect(body).to include "This order was automatically created for you."
         expect(body).to include "Unfortunately, not all products that you requested were available."
-        expect(body).to include "href=\"#{spree.order_url(order)}\""
       end
     end
 
@@ -39,13 +40,61 @@ describe SubscriptionMailer do
         body = SubscriptionMailer.deliveries.last.body.encoded
         expect(body).to include "This order was automatically created for you."
         expect(body).to_not include "Unfortunately, not all products that you requested were available."
-        expect(body).to include "href=\"#{spree.order_url(order)}\""
+      end
+    end
+
+    describe "linking to order page" do
+      let(:order_link_href) { "href=\"#{spree.order_url(order)}\"" }
+      let(:order_link_style) { "style='[^']+'" }
+
+      let(:shop) { create(:enterprise, allow_order_changes: true) }
+
+      let(:email) { SubscriptionMailer.deliveries.last }
+      let(:body) { email.body.encoded }
+
+      before do
+        SubscriptionMailer.placement_email(order, {}).deliver
+      end
+
+      context "when the customer has a user account" do
+        let(:customer) { create(:customer, enterprise: shop) }
+
+        it "provides link to make changes" do
+          expect(body).to match /<a #{order_link_href} #{order_link_style}>make changes<\/a>/
+          expect(body).to_not match /<a #{order_link_href} #{order_link_style}>view details of this order<\/a>/
+        end
+
+        context "when the distributor does not allow changes to the order" do
+          let(:shop) { create(:enterprise, allow_order_changes: false) }
+
+          it "provides link to view details" do
+            expect(body).to_not match /<a #{order_link_href} #{order_link_style}>make changes<\/a>/
+            expect(body).to match /<a #{order_link_href} #{order_link_style}>view details of this order<\/a>/
+          end
+        end
+      end
+
+      context "when the customer has no user account" do
+        let(:customer) { create(:customer, enterprise: shop, user: nil) }
+
+        it "does not provide link" do
+          expect(body).to_not match /#{order_link_href}/
+        end
+
+        context "when the distributor does not allow changes to the order" do
+          let(:shop) { create(:enterprise, allow_order_changes: false) }
+
+          it "does not provide link" do
+            expect(body).to_not match /#{order_link_href}/
+          end
+        end
       end
     end
   end
 
   describe "order confirmation" do
-    let(:subscription) { create(:subscription, with_items: true) }
+    let(:customer) { create(:customer) }
+    let(:subscription) { create(:subscription, customer: customer, with_items: true) }
     let(:proxy_order) { create(:proxy_order, subscription: subscription) }
     let!(:order) { proxy_order.initialise_order! }
 
@@ -58,7 +107,29 @@ describe SubscriptionMailer do
     it "sends the email" do
       body = SubscriptionMailer.deliveries.last.body.encoded
       expect(body).to include "This order was automatically placed for you"
-      expect(body).to include "href=\"#{spree.order_url(order)}\""
+    end
+
+    describe "linking to order page" do
+      let(:order_link_href) { "href=\"#{spree.order_url(order)}\"" }
+
+      let(:email) { SubscriptionMailer.deliveries.last }
+      let(:body) { email.body.encoded }
+
+      context "when the customer has a user account" do
+        let(:customer) { create(:customer) }
+
+        it "provides link to view details" do
+          expect(body).to match /#{order_link_href}/
+        end
+      end
+
+      context "when the customer has no user account" do
+        let(:customer) { create(:customer, user: nil) }
+
+        it "does not provide link" do
+          expect(body).to_not match /#{order_link_href}/
+        end
+      end
     end
   end
 
@@ -81,7 +152,8 @@ describe SubscriptionMailer do
   end
 
   describe "failed payment notification" do
-    let(:subscription) { create(:subscription, with_items: true) }
+    let(:customer) { create(:customer) }
+    let(:subscription) { create(:subscription, customer: customer, with_items: true) }
     let(:proxy_order) { create(:proxy_order, subscription: subscription) }
     let!(:order) { proxy_order.initialise_order! }
 
@@ -102,6 +174,29 @@ describe SubscriptionMailer do
       expect(body).to include strip_tags(details)
       expect(body).to include "This is a payment failure error"
     end
+
+    describe "linking to order page" do
+      let(:order_link_href) { "href=\"#{spree.order_url(order)}\"" }
+
+      let(:email) { SubscriptionMailer.deliveries.last }
+      let(:body) { email.body.encoded }
+
+      context "when the customer has a user account" do
+        let(:customer) { create(:customer) }
+
+        it "provides link to view details" do
+          expect(body).to match /#{order_link_href}/
+        end
+      end
+
+      context "when the customer has no user account" do
+        let(:customer) { create(:customer, user: nil) }
+
+        it "does not provide link" do
+          expect(body).to_not match /#{order_link_href}/
+        end
+      end
+    end
   end
 
   describe "order placement summary" do