From 5ce7905a332e271ce79d63ea42363552a5af83ed Mon Sep 17 00:00:00 2001 From: drummer83 Date: Sun, 31 Dec 2023 02:25:31 +0100 Subject: [PATCH 01/13] White labelling ALL customer facing emails White labelling added for Order: cancellation email, Order: invoice email, Shipment: shipped email, Subscriptions: authorize payment email, Subscriptions: placement email, Subscriptions: empty order email, Subscriptions: failed payment email White labelling existed already for Order: confirmation email, Subscriptions: order confirmation email --- app/mailers/payment_mailer.rb | 1 + app/mailers/spree/order_mailer.rb | 2 ++ app/mailers/spree/shipment_mailer.rb | 1 + app/mailers/subscription_mailer.rb | 3 +++ 4 files changed, 7 insertions(+) diff --git a/app/mailers/payment_mailer.rb b/app/mailers/payment_mailer.rb index f9819d252a..175e1da042 100644 --- a/app/mailers/payment_mailer.rb +++ b/app/mailers/payment_mailer.rb @@ -6,6 +6,7 @@ class PaymentMailer < ApplicationMailer def authorize_payment(payment) @payment = payment @order = @payment.order + @hide_ofn_navigation = @payment.order.distributor.hide_ofn_navigation subject = I18n.t('spree.payment_mailer.authorize_payment.subject', distributor: @order.distributor.name) I18n.with_locale valid_locale(@order.user) do diff --git a/app/mailers/spree/order_mailer.rb b/app/mailers/spree/order_mailer.rb index b396e79283..7ee64eab8a 100644 --- a/app/mailers/spree/order_mailer.rb +++ b/app/mailers/spree/order_mailer.rb @@ -11,6 +11,7 @@ module Spree def cancel_email(order_or_order_id, resend = false) @order = find_order(order_or_order_id) + @hide_ofn_navigation = @order.distributor.hide_ofn_navigation I18n.with_locale valid_locale(@order.user) do mail(to: @order.email, subject: mail_subject(t('spree.order_mailer.cancel_email.subject'), resend), @@ -51,6 +52,7 @@ module Spree def invoice_email(order_or_order_id, options = {}) @order = find_order(order_or_order_id) + @hide_ofn_navigation = @order.distributor.hide_ofn_navigation current_user = if options[:current_user_id].present? find_user(options[:current_user_id]) end diff --git a/app/mailers/spree/shipment_mailer.rb b/app/mailers/spree/shipment_mailer.rb index 7b9216335b..5955220d8d 100644 --- a/app/mailers/spree/shipment_mailer.rb +++ b/app/mailers/spree/shipment_mailer.rb @@ -4,6 +4,7 @@ module Spree class ShipmentMailer < ApplicationMailer def shipped_email(shipment, delivery:) @shipment = shipment.respond_to?(:id) ? shipment : Spree::Shipment.find(shipment) + @hide_ofn_navigation = @shipment.order.distributor.hide_ofn_navigation @delivery = delivery @order = @shipment.order subject = base_subject diff --git a/app/mailers/subscription_mailer.rb b/app/mailers/subscription_mailer.rb index b9aee15045..08b0031de4 100644 --- a/app/mailers/subscription_mailer.rb +++ b/app/mailers/subscription_mailer.rb @@ -19,6 +19,7 @@ class SubscriptionMailer < ApplicationMailer @type = 'empty' @changes = changes @order = order + @hide_ofn_navigation = @order.distributor.hide_ofn_navigation send_mail(order) end @@ -26,11 +27,13 @@ class SubscriptionMailer < ApplicationMailer @type = 'placement' @changes = changes @order = order + @hide_ofn_navigation = @order.distributor.hide_ofn_navigation send_mail(order) end def failed_payment_email(order) @order = order + @hide_ofn_navigation = @order.distributor.hide_ofn_navigation send_mail(order) end From 52ddb29dc7e91327a8e4f8c42f19035e39f7fac6 Mon Sep 17 00:00:00 2001 From: Konrad Date: Tue, 5 Aug 2025 15:27:42 +0200 Subject: [PATCH 02/13] Update user mailer specs to check that white labelling does not affect these emails Move tests to separate file for reuse in other emails Pass on :mail symbol and obtain the mail-object using public_send() to call it with different names --- spec/mailers/user_mailer_spec.rb | 34 ++++++++++++++----- .../shared_examples/email_header_examples.rb | 31 +++++++++++++++++ 2 files changed, 57 insertions(+), 8 deletions(-) create mode 100644 spec/support/shared_examples/email_header_examples.rb diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index ff8aec0761..29d092cecb 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -4,14 +4,19 @@ require 'spec_helper' RSpec.describe Spree::UserMailer do let(:user) { build(:user) } + let(:order) { build(:order_with_totals_and_distribution) } + + before { ActionMailer::Base.deliveries.clear } describe '#signup_confirmation' do + subject(:mail) { Spree::UserMailer.signup_confirmation(user) } + it "sends email when given a user" do - Spree::UserMailer.signup_confirmation(user).deliver_now + mail.deliver_now expect(ActionMailer::Base.deliveries.count).to eq(1) end - describe "user locale" do + context "user locale handling" do around do |example| original_default_locale = I18n.default_locale I18n.default_locale = 'pt' @@ -21,16 +26,21 @@ RSpec.describe Spree::UserMailer do it "sends email in user locale when user locale is defined" do user.locale = 'es' - Spree::UserMailer.signup_confirmation(user).deliver_now + mail.deliver_now 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_now + mail.deliver_now expect(ActionMailer::Base.deliveries.first.body).to include "Obrigada por juntar-se" end end + + context "white labelling" do + it_behaves_like 'email with inactive white labelling', :mail + it_behaves_like 'non-customer facing email with active white labelling', :mail + end end describe "#confirmation_instructions" do @@ -44,7 +54,6 @@ RSpec.describe Spree::UserMailer do context 'when the language is English' do it 'sends an email with the translated subject' do mail.deliver_now - expect(ActionMailer::Base.deliveries.first.subject).to include( "Please confirm your OFN account" ) @@ -56,19 +65,28 @@ RSpec.describe Spree::UserMailer do it 'sends an email with the translated subject' do mail.deliver_now - expect(ActionMailer::Base.deliveries.first.subject).to include( "Por favor, confirma tu cuenta de OFN" ) end end + + context "white labelling" do + it_behaves_like 'email with inactive white labelling', :mail + it_behaves_like 'non-customer facing email with active white labelling', :mail + end end # adapted from https://github.com/spree/spree_auth_devise/blob/70737af/spec/mailers/user_mailer_spec.rb describe '#reset_password_instructions' do - describe 'message contents' do - subject(:mail) { described_class.reset_password_instructions(user, nil).deliver_now } + subject(:mail) { described_class.reset_password_instructions(user, nil).deliver_now } + context "white labelling" do + it_behaves_like 'email with inactive white labelling', :mail + it_behaves_like 'non-customer facing email with active white labelling', :mail + end + + describe 'message contents' do context 'subject includes' do it 'translated devise instructions' do expect(mail.subject).to include "Reset password instructions" diff --git a/spec/support/shared_examples/email_header_examples.rb b/spec/support/shared_examples/email_header_examples.rb new file mode 100644 index 0000000000..c23d6403a3 --- /dev/null +++ b/spec/support/shared_examples/email_header_examples.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'email with inactive white labelling' do |mail| + it 'always displays the OFN header with logo' do + expect(public_send(mail).body).to include(ContentConfig.url_for(:logo)) + end +end + +RSpec.shared_examples 'non-customer facing email with active white labelling' do |mail| + context 'when hide OFN navigation is enabled for the distributor of the order' do + before do + allow(order.distributor).to receive(:hide_ofn_navigation).and_return(true) + end + + it 'still displays the OFN header with logo' do + expect(public_send(mail).body).to include(ContentConfig.url_for(:logo)) + end + end +end + +RSpec.shared_examples 'customer facing email with active white labelling' do |mail| + context 'when hide OFN navigation is enabled for the distributor of the order' do + before do + allow(order.distributor).to receive(:hide_ofn_navigation).and_return(true) + end + + it 'does not display the OFN header and logo' do + expect(public_send(mail).body).not_to include(ContentConfig.url_for(:logo)) + end + end +end From ab6a49e568131d4ea0fb76e7a9aed268b756afbc Mon Sep 17 00:00:00 2001 From: Konrad Date: Thu, 15 May 2025 16:29:28 +0200 Subject: [PATCH 03/13] Update order mailer specs to check that customer facing emails are white labelled, while shop facing emails are not Also make use of the newly separated shared_examples Including the invoice email was too tricky for me for now, sorry --- spec/mailers/order_mailer_spec.rb | 68 +++++++++++++++++-------------- 1 file changed, 37 insertions(+), 31 deletions(-) diff --git a/spec/mailers/order_mailer_spec.rb b/spec/mailers/order_mailer_spec.rb index 020f54c3b8..0bc416fb1e 100644 --- a/spec/mailers/order_mailer_spec.rb +++ b/spec/mailers/order_mailer_spec.rb @@ -4,19 +4,24 @@ require 'spec_helper' RSpec.describe Spree::OrderMailer do describe '#confirm_email_for_customer' do - subject(:email) { described_class.confirm_email_for_customer(order) } + subject(:mail) { described_class.confirm_email_for_customer(order) } let(:order) { build(:order_with_totals_and_distribution) } + context "white labelling" do + it_behaves_like 'email with inactive white labelling', :mail + it_behaves_like 'customer facing email with active white labelling', :mail + end + it 'renders the shared/_payment.html.haml partial' do - expect(email.body).to include('Payment summary') + expect(mail.body).to include('Payment summary') end context 'when the order has outstanding balance' do before { allow(order).to receive(:new_outstanding_balance) { 123 } } it 'renders the amount as money' do - expect(email.body).to include('$123') + expect(mail.body).to include('$123') end end @@ -24,18 +29,18 @@ RSpec.describe Spree::OrderMailer do before { allow(order).to receive(:new_outstanding_balance) { 0 } } it 'displays the payment status' do - expect(email.body).to include('NOT PAID') + expect(mail.body).to include('NOT PAID') end end context "when :from is not set explicitly" do it "falls back to spree config" do - expect(email.from).to eq [Spree::Config[:mails_from]] + expect(mail.from).to eq [Spree::Config[:mails_from]] end end it "doesn't aggressively escape double quotes body" do - expect(email.body).not_to include(""") + expect(mail.body).not_to include(""") end it "accepts an order id as an alternative to an Order object" do @@ -44,46 +49,37 @@ RSpec.describe Spree::OrderMailer do described_class.confirm_email_for_customer(order.id).deliver_now }.not_to raise_error end - - it "display the OFN header by default" do - expect(email.body).to include(ContentConfig.url_for(:logo)) - end - - context 'when hide OFN navigation is enabled for the distributor of the order' do - before do - allow(order.distributor).to receive(:hide_ofn_navigation).and_return(true) - end - - it 'does not display the OFN navigation' do - expect(email.body).not_to include(ContentConfig.url_for(:logo)) - end - end end describe '#confirm_email_for_shop' do - subject(:email) { described_class.confirm_email_for_shop(order) } + subject(:mail) { described_class.confirm_email_for_shop(order) } let(:order) { build(:order_with_totals_and_distribution) } + context "white labelling" do + it_behaves_like 'email with inactive white labelling', :mail + it_behaves_like 'non-customer facing email with active white labelling', :mail + end + it 'renders the shared/_payment.html.haml partial' do - expect(email.body).to include('Payment summary') + expect(mail.body).to include('Payment summary') end it "sets a reply-to of the customer email" do - expect(email.reply_to).to eq([order.email]) + expect(mail.reply_to).to eq([order.email]) end context 'when the order has outstanding balance' do before { allow(order).to receive(:new_outstanding_balance) { 123 } } it 'renders the amount as money' do - expect(email.body).to include('$123') + expect(mail.body).to include('$123') end end context 'when the order has no outstanding balance' do it 'displays the payment status' do - expect(email.body).to include('NOT PAID') + expect(mail.body).to include('NOT PAID') end end end @@ -151,6 +147,11 @@ RSpec.describe Spree::OrderMailer do expect(mail.to).to eq([distributor.contact.email]) end + context "white labelling" do + it_behaves_like 'email with inactive white labelling', :mail + it_behaves_like 'non-customer facing email with active white labelling', :mail + end + it "includes a link to the cancelled order in admin" do expect(mail.body).to match /#{admin_order_link_href}/ end @@ -169,6 +170,11 @@ RSpec.describe Spree::OrderMailer do expect(mail.to).to eq([order.email]) end + context "white labelling" do + it_behaves_like 'email with inactive white labelling', :mail + it_behaves_like 'customer facing email with active white labelling', :mail + end + it "displays the order number" do expect(mail.body).to include(order.number.to_s) end @@ -178,7 +184,7 @@ RSpec.describe Spree::OrderMailer do end end - describe "order confimation" do + describe "order confirmation" do let(:bill_address) { create(:address) } let(:distributor_address) { create(:address, address1: "distributor address", city: 'The Shire', zipcode: "1234") @@ -244,7 +250,7 @@ RSpec.describe Spree::OrderMailer do end describe "#invoice_email" do - subject(:email) { described_class.invoice_email(order) } + subject(:mail) { described_class.invoice_email(order) } let(:order) { create(:completed_order_with_totals) } let!(:invoice_data_generator){ InvoiceDataGenerator.new(order) } let!(:invoice){ @@ -261,17 +267,17 @@ RSpec.describe Spree::OrderMailer do allow(InvoiceRenderer).to receive(:new).and_return(renderer) end context "When invoices feature is not enabled" do - it "should call the invoice render with order as argument" do + it "should call the invoice renderer with order as argument" do expect(generator).not_to receive(:generate_or_update_latest_invoice) expect(order).not_to receive(:invoices) expect(renderer).to receive(:render_to_string).with(order, nil).and_return("invoice") expect { - email.deliver_now + mail.deliver_now }.not_to raise_error expect(deliveries.count).to eq(1) expect(deliveries.first.attachments.count).to eq(1) expect(deliveries.first.attachments.first.filename).to eq(attachment_filename) - expect(email.reply_to).to eq([order.distributor.contact.email]) + expect(mail.reply_to).to eq([order.distributor.contact.email]) end end @@ -280,7 +286,7 @@ RSpec.describe Spree::OrderMailer do expect(generator).to receive(:generate_or_update_latest_invoice) expect(order).to receive(:invoices).and_return([invoice]) expect(renderer).to receive(:render_to_string).with(invoice.presenter, nil) - email.deliver_now + mail.deliver_now end end end From 183cbecef6fffb1cb1137529e83b44f33bf80219 Mon Sep 17 00:00:00 2001 From: Konrad Date: Sun, 25 May 2025 18:43:15 +0200 Subject: [PATCH 04/13] Update shipment mailer specs to check that customer facing emails are white labelled (there are no shop facing emails here) Make use of the newly separated shared_examples --- spec/mailers/shipment_mailer_spec.rb | 29 +++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/spec/mailers/shipment_mailer_spec.rb b/spec/mailers/shipment_mailer_spec.rb index bf234ce496..c1942b722f 100644 --- a/spec/mailers/shipment_mailer_spec.rb +++ b/spec/mailers/shipment_mailer_spec.rb @@ -3,8 +3,9 @@ require 'spec_helper' RSpec.describe Spree::ShipmentMailer do + let(:order) { build(:order_with_distributor) } + let(:shipment) do - order = build(:order_with_distributor) product = build(:product, name: %{The "BEST" product}) variant = build(:variant, product:) line_item = build(:line_item, variant:, order:, quantity: 1, price: 5) @@ -13,18 +14,26 @@ RSpec.describe Spree::ShipmentMailer do allow(shipment).to receive_messages(tracking_url: "TRACK_ME") shipment end + + let(:shipment_email) { described_class.shipped_email(shipment, delivery: true) } + let(:picked_up_email) { described_class.shipped_email(shipment, delivery: false) } let(:distributor) { shipment.order.distributor } context ":from not set explicitly" do it "falls back to spree config" do - message = Spree::ShipmentMailer.shipped_email(shipment, delivery: true) - expect(message.from).to eq [Spree::Config[:mails_from]] + expect(shipment_email.from).to eq [Spree::Config[:mails_from]] end end + context "white labelling" do + it_behaves_like 'email with inactive white labelling', :shipment_email + it_behaves_like 'customer facing email with active white labelling', :shipment_email + it_behaves_like 'email with inactive white labelling', :picked_up_email + it_behaves_like 'customer facing email with active white labelling', :picked_up_email + end + # Regression test for #2196 it "doesn't include out of stock in the email body" do - shipment_email = Spree::ShipmentMailer.shipped_email(shipment, delivery: true) expect(shipment_email.body).not_to include(%{Out of Stock}) end @@ -36,33 +45,27 @@ RSpec.describe Spree::ShipmentMailer do end it "includes the 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") end it "includes the distributor's name in the body" do - shipment_email = Spree::ShipmentMailer.shipped_email(shipment, delivery: true) expect(shipment_email.body).to include("Your order from #{distributor.name} has been shipped") end it "picked_up email includes different text in body" do text = "Your order from #{distributor.name} has been picked-up" - picked_up_email = Spree::ShipmentMailer.shipped_email(shipment, delivery: false) expect(picked_up_email.body).to include(text) end 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(picked_up_email.subject).to include("#{distributor.name} Pick up Notification") end it "picked_up email has as the reply to email as the distributor" do - shipment_email = Spree::ShipmentMailer.shipped_email(shipment, delivery: false) - expect(shipment_email.reply_to).to eq([distributor.contact.email]) + expect(picked_up_email.reply_to).to eq([distributor.contact.email]) end - it "shipment_email email has as the reply to email as the distributor" do - shipment_email = Spree::ShipmentMailer.shipped_email(shipment, delivery: true) + it "shipment_email has as the reply to email as the distributor" do expect(shipment_email.reply_to).to eq([distributor.contact.email]) end end From 0de4f2f5961f7aefae5b64d06411882301f81736 Mon Sep 17 00:00:00 2001 From: Konrad Date: Mon, 26 May 2025 18:36:39 +0200 Subject: [PATCH 05/13] Update payment mailer specs to check that customer facing emails are white labelled, while shop facing emails are not Also make use of the newly separated shared_examples --- spec/mailers/payment_mailer_spec.rb | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/spec/mailers/payment_mailer_spec.rb b/spec/mailers/payment_mailer_spec.rb index 1c520fbc19..f36ec9ea6c 100644 --- a/spec/mailers/payment_mailer_spec.rb +++ b/spec/mailers/payment_mailer_spec.rb @@ -14,31 +14,41 @@ RSpec.describe PaymentMailer do let(:order) { create(:completed_order_with_totals) } context "authorize payment email" do - subject(:email) { described_class.authorize_payment(payment) } + subject(:mail) { described_class.authorize_payment(payment) } it "includes the distributor's name in the subject" do - expect(email.subject).to include("authorize your payment to #{order.distributor.name}") + expect(mail.subject).to include("authorize your payment to #{order.distributor.name}") end it "sets a reply-to of the customer email" do - expect(email.reply_to).to eq([order.distributor.contact.email]) + expect(mail.reply_to).to eq([order.distributor.contact.email]) + end + + context "white labelling" do + it_behaves_like 'email with inactive white labelling', :mail + it_behaves_like 'customer facing email with active white labelling', :mail end it "includes a link to authorize the payment" do link = "http://test.host/payments/#{payment.id}/authorize" - expect(email.body).to have_link link, href: link + expect(mail.body).to have_link link, href: link end end context "authorization required email" do - subject(:email) { described_class.authorization_required(payment) } + subject(:mail) { described_class.authorization_required(payment) } it "includes the distributor's name in the subject" do - expect(email.subject).to include("A payment requires authorization from the customer") + expect(mail.subject).to include("A payment requires authorization from the customer") end it "sets a reply-to of the customer email" do - expect(email.reply_to).to eq([order.email]) + expect(mail.reply_to).to eq([order.email]) + end + + context "white labelling" do + it_behaves_like 'email with inactive white labelling', :mail + it_behaves_like 'non-customer facing email with active white labelling', :mail end end end From 89453ec75805301a74dddea46990658a42d0a80f Mon Sep 17 00:00:00 2001 From: Konrad Date: Wed, 28 May 2025 10:36:37 +0200 Subject: [PATCH 06/13] Update subscription mailer specs to check that customer facing emails are white labelled, while shop facing emails are not Also make use of the newly separated shared_examples Made the check for link to order page more general, because on my system a double quote was expected but a single quote was generated --- spec/mailers/subscription_mailer_spec.rb | 144 ++++++++++++----------- 1 file changed, 78 insertions(+), 66 deletions(-) diff --git a/spec/mailers/subscription_mailer_spec.rb b/spec/mailers/subscription_mailer_spec.rb index f79f81e9e6..bdcb32ffdc 100644 --- a/spec/mailers/subscription_mailer_spec.rb +++ b/spec/mailers/subscription_mailer_spec.rb @@ -5,8 +5,8 @@ require 'spec_helper' RSpec.describe SubscriptionMailer do include ActionView::Helpers::SanitizeHelper - describe '#placement_email' do - subject(:email) { SubscriptionMailer.placement_email(order, changes) } + describe '#placement_email (customer)' do + subject(:mail) { described_class.placement_email(order, changes) } let(:changes) { {} } let(:shop) { create(:enterprise) } @@ -15,11 +15,16 @@ RSpec.describe SubscriptionMailer do let(:proxy_order) { create(:proxy_order, subscription:) } let!(:order) { proxy_order.initialise_order! } + context "white labelling" do + it_behaves_like 'email with inactive white labelling', :mail + it_behaves_like 'customer facing email with active white labelling', :mail + end + context "when changes have been made to the order" do before { changes[order.line_items.first.id] = 2 } it "sends the email, which notifies the customer of changes made" do - expect { email.deliver_now }.to change { SubscriptionMailer.deliveries.count }.by(1) + expect { mail.deliver_now }.to change { SubscriptionMailer.deliveries.count }.by(1) body = SubscriptionMailer.deliveries.last.body.encoded @@ -30,7 +35,7 @@ RSpec.describe SubscriptionMailer do context "and changes have not been made to the order" do it "sends the email" do - expect { email.deliver_now }.to change { SubscriptionMailer.deliveries.count }.by(1) + expect { mail.deliver_now }.to change { SubscriptionMailer.deliveries.count }.by(1) body = SubscriptionMailer.deliveries.last.body.encoded @@ -46,7 +51,7 @@ RSpec.describe SubscriptionMailer do let(:content) { Capybara::Node::Simple.new(body) } let(:body) { SubscriptionMailer.deliveries.last.body.encoded } - before { email.deliver_now } + before { mail.deliver_now } context "when the customer has a user account" do let(:customer) { create(:customer, enterprise: shop) } @@ -87,7 +92,7 @@ RSpec.describe SubscriptionMailer do before { allow(order).to receive(:new_outstanding_balance) { 123 } } it 'renders the amount as money' do - expect(email.body).to include('$123') + expect(mail.body).to include('$123') end end @@ -95,13 +100,13 @@ RSpec.describe SubscriptionMailer do before { allow(order).to receive(:new_outstanding_balance) { 0 } } it 'displays the payment status' do - expect(email.body).to include('NOT PAID') + expect(mail.body).to include('NOT PAID') end end end - describe '#confirmation_email' do - subject(:email) { SubscriptionMailer.confirmation_email(order) } + describe '#confirmation_email (customer)' do + subject(:mail) { described_class.confirmation_email(order) } let(:customer) { create(:customer) } let(:subscription) { create(:subscription, customer:, with_items: true) } @@ -110,14 +115,15 @@ RSpec.describe SubscriptionMailer do let(:user) { order.user } it "sends the email" do - expect { email.deliver_now }.to change{ SubscriptionMailer.deliveries.count }.by(1) + expect { mail.deliver_now }.to change{ SubscriptionMailer.deliveries.count }.by(1) body = SubscriptionMailer.deliveries.last.body.encoded expect(body).to include "This order was automatically placed for you" end - it "display the OFN header by default" do - expect(email.body).to include(ContentConfig.url_for(:logo)) + context "white labelling" do + it_behaves_like 'email with inactive white labelling', :mail + it_behaves_like 'customer facing email with active white labelling', :mail end describe "linking to order page" do @@ -127,7 +133,7 @@ RSpec.describe SubscriptionMailer do let(:customer) { create(:customer) } it "provides link to view details" do - expect(email.body.encoded).to include(order_url(order)) + expect(mail.body.encoded).to include(order_url(order)) end end @@ -135,7 +141,7 @@ RSpec.describe SubscriptionMailer do let(:customer) { create(:customer, user: nil) } it "does not provide link" do - expect(email.body).not_to match /#{order_link_href}/ + expect(mail.body).not_to match /#{order_link_href}/ end end end @@ -144,7 +150,7 @@ RSpec.describe SubscriptionMailer do before { allow(order).to receive(:new_outstanding_balance) { 123 } } it 'renders the amount as money' do - expect(email.body).to include('$123') + expect(mail.body).to include('$123') end end @@ -152,40 +158,36 @@ RSpec.describe SubscriptionMailer do before { allow(order).to receive(:new_outstanding_balance) { 0 } } it 'displays the payment status' do - expect(email.body).to include('NOT PAID') - end - end - - context 'when hide OFN navigation is enabled for the distributor of the order' do - before do - allow(order.distributor).to receive(:hide_ofn_navigation).and_return(true) - end - - it 'does not display the OFN navigation' do - expect(email.body).not_to include(ContentConfig.url_for(:logo)) + expect(mail.body).to include('NOT PAID') end end end - describe "empty order notification" do + describe "#empty_order_email (customer)" do + subject(:mail) { described_class.empty_email(order, {}) } + let(:subscription) { create(:subscription, with_items: true) } let(:proxy_order) { create(:proxy_order, subscription:) } + let(:distributor) { create(:distributor_enterprise) } let!(:order) { proxy_order.initialise_order! } - before do - expect do - SubscriptionMailer.empty_email(order, {}).deliver_now - end.to change{ SubscriptionMailer.deliveries.count }.by(1) + context "white labelling" do + it_behaves_like 'email with inactive white labelling', :mail + it_behaves_like 'customer facing email with active white labelling', :mail end it "sends the email" do + expect { mail.deliver_now }.to change{ SubscriptionMailer.deliveries.count }.by(1) + body = SubscriptionMailer.deliveries.last.body.encoded expect(body).to include "We tried to place a new order with" expect(body).to include "Unfortunately, none of products that you ordered were available" end end - describe "failed payment notification" do + describe "#failed_payment_email (customer)" do + subject(:mail) { described_class.failed_payment_email(order) } + let(:customer) { create(:customer) } let(:subscription) { create(:subscription, customer:, with_items: true) } let(:proxy_order) { create(:proxy_order, subscription:) } @@ -193,13 +195,16 @@ RSpec.describe SubscriptionMailer do before do order.errors.add(:base, "This is a payment failure error") + end - expect do - SubscriptionMailer.failed_payment_email(order).deliver_now - end.to change{ SubscriptionMailer.deliveries.count }.by(1) + context "white labelling" do + it_behaves_like 'email with inactive white labelling', :mail + it_behaves_like 'customer facing email with active white labelling', :mail end it "sends the email" do + expect { mail.deliver_now }.to change{ SubscriptionMailer.deliveries.count }.by(1) + body = strip_tags(SubscriptionMailer.deliveries.last.body.encoded) expect(body).to include 'We tried to process a payment, but had some problems...' email_so_failed_payment_explainer_html = "The payment for your subscription with %s" \ @@ -214,10 +219,8 @@ RSpec.describe SubscriptionMailer do end describe "linking to order page" do - let(:order_link_href) { "href=\"#{order_url(order)}\"" } - - let(:email) { SubscriptionMailer.deliveries.last } - let(:body) { email.body.encoded } + let(:order_link_href) { "href=['\"]#{order_url(order)}['\"]" } + let(:body) { mail.body.encoded } context "when the customer has a user account" do let(:customer) { create(:customer) } @@ -237,28 +240,37 @@ RSpec.describe SubscriptionMailer do end end - describe "order placement summary" do + describe "#order_placement_summary_email (hub)" do + subject(:mail) { described_class.placement_summary_email(summary) } + let!(:shop) { create(:enterprise, :with_logo_image) } let!(:summary) { double(:summary, shop_id: shop.id) } let(:body) { strip_tags(SubscriptionMailer.deliveries.last.body.encoded) } let(:scope) { "subscription_mailer" } + let(:order) { build(:order_with_distributor) } before do allow(summary).to receive(:unrecorded_ids) { [] } allow(summary).to receive(:subscription_issues) { [] } + allow(summary).to receive(:order_count) { 37 } + allow(summary).to receive(:issue_count) { 0 } + allow(summary).to receive(:issues) { {} } + end + + it "renders the shop's logo" do + mail.deliver_now + expect(SubscriptionMailer.deliveries.last.body).to include "logo.png" + end + + context "white labelling" do + it_behaves_like 'email with inactive white labelling', :mail + it_behaves_like 'non-customer facing email with active white labelling', :mail end context "when no issues were encountered while processing subscriptions" do - before do - allow(summary).to receive(:order_count) { 37 } - allow(summary).to receive(:issue_count) { 0 } - allow(summary).to receive(:issues) { {} } - end - it "sends the email, which notifies the enterprise that all orders " \ "were successfully processed" do - SubscriptionMailer.placement_summary_email(summary).deliver_now - + mail.deliver_now expect(body).to include("Below is a summary of the subscription orders " \ "that have just been placed for %s." % shop.name) expect(body).to include("A total of %d subscriptions were marked " \ @@ -266,11 +278,6 @@ RSpec.describe SubscriptionMailer do expect(body).to include 'All were processed successfully.' expect(body).not_to include 'Details of the issues encountered are provided below.' end - - it "renders the shop's logo" do - SubscriptionMailer.placement_summary_email(summary).deliver_now - expect(SubscriptionMailer.deliveries.last.body).to include "logo.png" - end end context "when some issues were encountered while processing subscriptions" do @@ -289,7 +296,7 @@ RSpec.describe SubscriptionMailer do context "when no unrecorded issues are present" do it "sends the email, which notifies the enterprise that some issues were encountered" do - SubscriptionMailer.placement_summary_email(summary).deliver_now + mail.deliver_now expect(body).to include("Below is a summary of the subscription orders " \ "that have just been placed for %s." % shop.name) expect(body).to include("A total of %d subscriptions were marked " \ @@ -320,7 +327,7 @@ RSpec.describe SubscriptionMailer do it "sends the email, which notifies the enterprise that some issues were encountered" do expect(summary).to receive(:orders_affected_by).with(:other) { [order3, order4] } - SubscriptionMailer.placement_summary_email(summary).deliver_now + mail.deliver_now expect(body).to include("Error Encountered (%d orders)" % 2) expect(body).to include 'Automatic processing of these orders failed due to an error. ' \ 'The error has been listed where possible.' @@ -346,7 +353,7 @@ RSpec.describe SubscriptionMailer do allow(summary).to receive(:issue_count) { 2 } allow(summary).to receive(:issues) { { changes: { 1 => nil, 2 => nil } } } allow(summary).to receive(:orders_affected_by) { [order1, order2] } - SubscriptionMailer.placement_summary_email(summary).deliver_now + mail.deliver_now end it "sends the email, which notifies the enterprise that some issues were encountered" do @@ -370,24 +377,29 @@ RSpec.describe SubscriptionMailer do end end - describe "order confirmation summary" do + describe "#order_confirmation_summary_email (hub)" do + subject(:mail) { SubscriptionMailer.confirmation_summary_email(summary) } let!(:shop) { create(:enterprise) } let!(:summary) { double(:summary, shop_id: shop.id) } let(:body) { strip_tags(SubscriptionMailer.deliveries.last.body.encoded) } let(:scope) { "subscription_mailer" } + let(:order) { build(:order_with_distributor) } before do allow(summary).to receive(:unrecorded_ids) { [] } allow(summary).to receive(:subscription_issues) { [] } + allow(summary).to receive(:order_count) { 37 } + allow(summary).to receive(:issue_count) { 0 } + allow(summary).to receive(:issues) { {} } + end + + context "white labelling" do + it_behaves_like 'email with inactive white labelling', :mail + it_behaves_like 'non-customer facing email with active white labelling', :mail end context "when no issues were encountered while processing subscriptions" do - before do - allow(summary).to receive(:order_count) { 37 } - allow(summary).to receive(:issue_count) { 0 } - allow(summary).to receive(:issues) { {} } - SubscriptionMailer.confirmation_summary_email(summary).deliver_now - end + before { mail.deliver_now } it "sends the email, which notifies the enterprise " \ "that all orders were successfully processed" do @@ -416,7 +428,7 @@ RSpec.describe SubscriptionMailer do context "when no unrecorded issues are present" do it "sends the email, which notifies the enterprise that some issues were encountered" do - SubscriptionMailer.confirmation_summary_email(summary).deliver_now + mail.deliver_now expect(body).to include("Below is a summary of the subscription orders " \ "that have just been finalised for %s." % shop.name) expect(body).to include("A total of %d subscriptions were marked " \ @@ -447,7 +459,7 @@ RSpec.describe SubscriptionMailer do it "sends the email, which notifies the enterprise that some issues were encountered" do expect(summary).to receive(:orders_affected_by).with(:other) { [order3, order4] } - SubscriptionMailer.confirmation_summary_email(summary).deliver_now + mail.deliver_now expect(body).to include("Failed Payment (%d orders)" % 2) expect(body).to include 'Automatic processing of payment for these orders failed ' \ 'due to an error. The error has been listed where possible.' @@ -473,7 +485,7 @@ RSpec.describe SubscriptionMailer do allow(summary).to receive(:issue_count) { 2 } allow(summary).to receive(:issues) { { changes: { 1 => nil, 2 => nil } } } allow(summary).to receive(:orders_affected_by) { [order1, order2] } - SubscriptionMailer.confirmation_summary_email(summary).deliver_now + mail.deliver_now end it "sends the email, which notifies the enterprise that some issues were encountered" do From 5957d99812158935fa10fb3cd23c39891e5a3fe9 Mon Sep 17 00:00:00 2001 From: Konrad Date: Tue, 24 Jun 2025 17:56:03 +0200 Subject: [PATCH 07/13] Update enterprise mailer specs to check that enterprise facing emails remain unaffected by white labelling (there are no customer facing emails here) Also make use of the newly separated shared_examples --- spec/mailers/enterprise_mailer_spec.rb | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/spec/mailers/enterprise_mailer_spec.rb b/spec/mailers/enterprise_mailer_spec.rb index 797dd78ddc..25acda034a 100644 --- a/spec/mailers/enterprise_mailer_spec.rb +++ b/spec/mailers/enterprise_mailer_spec.rb @@ -4,9 +4,10 @@ require 'spec_helper' RSpec.describe EnterpriseMailer do let(:enterprise) { build(:enterprise, name: "Fred's Farm") } + let(:order) { build(:order_with_distributor) } describe "#welcome" do - subject(:mail) { EnterpriseMailer.welcome(enterprise) } + subject(:mail) { described_class.welcome(enterprise) } it "sends a welcome email when given an enterprise" do expect(mail.subject) @@ -16,10 +17,15 @@ RSpec.describe EnterpriseMailer do it "does not set a reply-to email" do expect(mail.reply_to).to eq nil end + + context "white labelling" do + it_behaves_like 'email with inactive white labelling', :mail + it_behaves_like 'non-customer facing email with active white labelling', :mail + end end describe "#manager_invitation" do - subject(:mail) { EnterpriseMailer.manager_invitation(enterprise, user) } + subject(:mail) { described_class.manager_invitation(enterprise, user) } let(:user) { build(:user) } it "should send a manager invitation email when given an enterprise and user" do @@ -29,5 +35,10 @@ RSpec.describe EnterpriseMailer do it "sets a reply-to of the enterprise email" do expect(mail.reply_to).to eq([enterprise.contact.email]) end + + context "white labelling" do + it_behaves_like 'email with inactive white labelling', :mail + it_behaves_like 'non-customer facing email with active white labelling', :mail + end end end From 0079ed219b5fe802f44fe2b5a1d8fb393b1d6a62 Mon Sep 17 00:00:00 2001 From: Konrad Date: Tue, 24 Jun 2025 18:12:32 +0200 Subject: [PATCH 08/13] Update producer mailer specs to check that this producer facing email remains unaffected by white labelling (there are no customer facing emails here) Also make use of the newly separated shared_examples --- spec/mailers/producer_mailer_spec.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/spec/mailers/producer_mailer_spec.rb b/spec/mailers/producer_mailer_spec.rb index b909a31933..27711d711b 100644 --- a/spec/mailers/producer_mailer_spec.rb +++ b/spec/mailers/producer_mailer_spec.rb @@ -66,6 +66,11 @@ RSpec.describe ProducerMailer do expect(mail.reply_to).to eq [order_cycle.coordinator.contact.email] end + context "white labelling" do + it_behaves_like 'email with inactive white labelling', :mail + it_behaves_like 'non-customer facing email with active white labelling', :mail + end + it "includes the pickup time for each distributor" do expect(mail.body.encoded).to include "#{d1.name} (Tue, 23rd Dec)" end From 593bd89095890454bc239d9dfc80899c0a936210 Mon Sep 17 00:00:00 2001 From: Konrad Date: Tue, 24 Jun 2025 18:29:58 +0200 Subject: [PATCH 09/13] Update test mailer specs to check that this super admin facing email remains unaffected by white labelling (there are no user or customer facing emails here) Also make use of the newly separated shared_examples --- spec/mailers/test_mailer_spec.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/spec/mailers/test_mailer_spec.rb b/spec/mailers/test_mailer_spec.rb index 191f84a344..0571640d10 100644 --- a/spec/mailers/test_mailer_spec.rb +++ b/spec/mailers/test_mailer_spec.rb @@ -3,7 +3,9 @@ require 'spec_helper' RSpec.describe Spree::TestMailer do + subject(:mail) { described_class.test_email(order) } let(:user) { create(:user) } + let(:order) { build(:order_with_distributor) } context ":from not set explicitly" do it "falls back to spree config" do @@ -18,4 +20,9 @@ RSpec.describe Spree::TestMailer do Spree::TestMailer.test_email(user.id).deliver_now }.not_to raise_error end + + context "white labelling" do + it_behaves_like 'email with inactive white labelling', :mail + it_behaves_like 'non-customer facing email with active white labelling', :mail + end end From 29a76fd721040b58b939d996df76a52515f5ab65 Mon Sep 17 00:00:00 2001 From: Konrad Date: Fri, 11 Jul 2025 09:54:13 +0200 Subject: [PATCH 10/13] Update report mailer specs to check that this enterprise facing email remains unaffected by white labelling (there are no customer facing emails here) Also make use of the newly separated shared_examples --- spec/mailers/report_mailer_spec.rb | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/spec/mailers/report_mailer_spec.rb b/spec/mailers/report_mailer_spec.rb index 92bf7032c1..2505825373 100644 --- a/spec/mailers/report_mailer_spec.rb +++ b/spec/mailers/report_mailer_spec.rb @@ -4,25 +4,31 @@ require 'spec_helper' RSpec.describe ReportMailer do describe "#report_ready" do - subject(:email) { + subject(:mail) { ReportMailer.with( to: "current_user@example.net", blob:, ).report_ready } let(:blob) { ReportBlob.create_locally!("customers.csv", "report content") } + let(:order) { build(:order_with_distributor) } + + context "white labelling" do + it_behaves_like 'email with inactive white labelling', :mail + it_behaves_like 'non-customer facing email with active white labelling', :mail + end it "notifies about a report" do - expect(email.subject).to eq "Report ready" - expect(email.body).to have_content "Report ready for download" + expect(mail.subject).to eq "Report ready" + expect(mail.body).to have_content "Report ready for download" end it "notifies the user" do - expect(email.to).to eq ["current_user@example.net"] + expect(mail.to).to eq ["current_user@example.net"] end it "contains a download link" do - expect(email.body).to have_link( + expect(mail.body).to have_link( "customers.csv", href: %r"^http://test\.host/rails/active_storage/disk/.*/customers\.csv$" ) From 3a75c3446ede62c2571ba41a5bdf38c67c1f61db Mon Sep 17 00:00:00 2001 From: Konrad Date: Fri, 11 Jul 2025 10:46:17 +0200 Subject: [PATCH 11/13] Update backorder mailer specs to check that these hub facing emails remain unaffected by white labelling (there are no customer facing emails here) Also make use of the newly separated shared_examples --- spec/mailers/backorder_mailer_spec.rb | 29 +++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/spec/mailers/backorder_mailer_spec.rb b/spec/mailers/backorder_mailer_spec.rb index 0073c423ec..4c6858d9a9 100644 --- a/spec/mailers/backorder_mailer_spec.rb +++ b/spec/mailers/backorder_mailer_spec.rb @@ -6,18 +6,30 @@ RSpec.describe BackorderMailer do let(:order) { create(:completed_order_with_totals) } describe "#backorder_failed" do + subject(:mail) { described_class.backorder_failed(order) } + it "notifies the owner" do order.distributor.owner.email = "jane@example.net" BackorderMailer.backorder_failed(order).deliver_now - mail = ActionMailer::Base.deliveries.first - expect(mail.to).to eq ["jane@example.net"] - expect(mail.subject).to eq "An automatic backorder failed" + first_mail = ActionMailer::Base.deliveries.first + expect(first_mail.to).to eq ["jane@example.net"] + expect(first_mail.subject).to eq "An automatic backorder failed" + end + + context "white labelling" do + it_behaves_like 'email with inactive white labelling', :mail + it_behaves_like 'non-customer facing email with active white labelling', :mail end end describe "#backorder_incomplete" do + subject(:mail) { + described_class.backorder_incomplete( + user, distributor, order_cycle, order_id + ) + } let(:user) { build(:user, email: "jane@example.net") } let(:distributor) { build(:enterprise) } let(:order_cycle) { build(:order_cycle) } @@ -26,9 +38,14 @@ RSpec.describe BackorderMailer do it "notifies the owner" do BackorderMailer.backorder_incomplete(user, distributor, order_cycle, order_id).deliver_now - mail = ActionMailer::Base.deliveries.first - expect(mail.to).to eq ["jane@example.net"] - expect(mail.subject).to eq "An automatic backorder failed to complete" + first_mail = ActionMailer::Base.deliveries.first + expect(first_mail.to).to eq ["jane@example.net"] + expect(first_mail.subject).to eq "An automatic backorder failed to complete" + end + + context "white labelling" do + it_behaves_like 'email with inactive white labelling', :mail + it_behaves_like 'non-customer facing email with active white labelling', :mail end end end From 4d6231105f7a087fea8640a99cbf13a864e02551 Mon Sep 17 00:00:00 2001 From: David Cook Date: Mon, 11 Aug 2025 14:21:20 +1000 Subject: [PATCH 12/13] Assign attribute directly instead of mocking It's better to set a variable the same way the real code would Co-authored-by: Maikel --- spec/support/shared_examples/email_header_examples.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/support/shared_examples/email_header_examples.rb b/spec/support/shared_examples/email_header_examples.rb index c23d6403a3..0b68a6b127 100644 --- a/spec/support/shared_examples/email_header_examples.rb +++ b/spec/support/shared_examples/email_header_examples.rb @@ -9,7 +9,7 @@ end RSpec.shared_examples 'non-customer facing email with active white labelling' do |mail| context 'when hide OFN navigation is enabled for the distributor of the order' do before do - allow(order.distributor).to receive(:hide_ofn_navigation).and_return(true) + order.distributor.hide_ofn_navigation = true end it 'still displays the OFN header with logo' do @@ -21,7 +21,7 @@ end RSpec.shared_examples 'customer facing email with active white labelling' do |mail| context 'when hide OFN navigation is enabled for the distributor of the order' do before do - allow(order.distributor).to receive(:hide_ofn_navigation).and_return(true) + order.distributor.hide_ofn_navigation = true end it 'does not display the OFN header and logo' do From e814fdd4474c2234011569e23ee00491e34fc6d0 Mon Sep 17 00:00:00 2001 From: Konrad Date: Mon, 29 Dec 2025 12:36:50 +0100 Subject: [PATCH 13/13] Remove test for active white labeling in test email as proposed by David --- spec/mailers/test_mailer_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/mailers/test_mailer_spec.rb b/spec/mailers/test_mailer_spec.rb index 0571640d10..314be75f74 100644 --- a/spec/mailers/test_mailer_spec.rb +++ b/spec/mailers/test_mailer_spec.rb @@ -23,6 +23,5 @@ RSpec.describe Spree::TestMailer do context "white labelling" do it_behaves_like 'email with inactive white labelling', :mail - it_behaves_like 'non-customer facing email with active white labelling', :mail end end