Skip to content

Commit

Permalink
Switch the mailers to be invoked using ids, not model objects. This m…
Browse files Browse the repository at this point in the history
…akes it easier to support asynchronous mail jobs, as only simple ids need to be persisted.

Fixes #2808
  • Loading branch information
petergoldstein authored and radar committed Apr 10, 2013
1 parent 0bb5cd6 commit 8d90ae1
Show file tree
Hide file tree
Showing 10 changed files with 48 additions and 17 deletions.
2 changes: 1 addition & 1 deletion backend/app/controllers/spree/admin/orders_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ def fire
end

def resend
OrderMailer.confirm_email(@order, true).deliver
OrderMailer.confirm_email(@order.id, true).deliver
flash[:success] = t(:order_email_resent)

redirect_to :back
Expand Down
12 changes: 6 additions & 6 deletions core/app/mailers/spree/order_mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,19 @@ def from_address
end

def confirm_email(order, resend = false)
@order = order
@order = order.respond_to?(:id) ? order : Spree::Order.find(order)
subject = (resend ? "[#{t(:resend).upcase}] " : '')
subject += "#{Spree::Config[:site_name]} #{t('order_mailer.confirm_email.subject')} ##{order.number}"
mail(to: order.email,
subject += "#{Spree::Config[:site_name]} #{t('order_mailer.confirm_email.subject')} ##{@order.number}"
mail(to: @order.email,
from: from_address,
subject: subject)
end

def cancel_email(order, resend = false)
@order = order
@order = order.respond_to?(:id) ? order : Spree::Order.find(order)
subject = (resend ? "[#{t(:resend).upcase}] " : '')
subject += "#{Spree::Config[:site_name]} #{t('order_mailer.cancel_email.subject')} ##{order.number}"
mail(to: order.email,
subject += "#{Spree::Config[:site_name]} #{t('order_mailer.cancel_email.subject')} ##{@order.number}"
mail(to: @order.email,
from: from_address,
subject: subject)
end
Expand Down
6 changes: 3 additions & 3 deletions core/app/mailers/spree/shipment_mailer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ def from_address
end

def shipped_email(shipment, resend = false)
@shipment = shipment
@shipment = shipment.respond_to?(:id) ? shipment : Spree::Shipment.find(shipment)
subject = (resend ? "[#{t(:resend).upcase}] " : '')
subject += "#{Spree::Config[:site_name]} #{t('shipment_mailer.shipped_email.subject')} ##{shipment.order.number}"
mail(to: shipment.order.email,
subject += "#{Spree::Config[:site_name]} #{t('shipment_mailer.shipped_email.subject')} ##{@shipment.order.number}"
mail(to: @shipment.order.email,
from: from_address,
subject: subject)
end
Expand Down
4 changes: 2 additions & 2 deletions core/app/models/spree/order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ def finalize!

def deliver_order_confirmation_email
begin
OrderMailer.confirm_email(self).deliver
OrderMailer.confirm_email(self.id).deliver
rescue Exception => e
logger.error("#{e.class.name}: #{e.message}")
logger.error(e.backtrace * "\n")
Expand Down Expand Up @@ -563,7 +563,7 @@ def has_available_payment
def after_cancel
shipments.each { |shipment| shipment.cancel! }

OrderMailer.cancel_email(self).deliver
OrderMailer.cancel_email(self.id).deliver
self.payment_state = 'credit_owed' unless shipped?
end

Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/shipment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ def after_ship
end

def send_shipped_email
ShipmentMailer.shipped_email(self).deliver
ShipmentMailer.shipped_email(self.id).deliver
end

def ensure_correct_adjustment
Expand Down
14 changes: 14 additions & 0 deletions core/spec/mailers/order_mailer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,20 @@
confirmation_email.body.should_not include(""")
end

it "confirm_email accepts an order id as an alternative to an Order object" do
Spree::Order.should_receive(:find).with(order.id).and_return(order)
lambda {
confirmation_email = Spree::OrderMailer.confirm_email(order.id)
}.should_not raise_error
end

it "cancel_email accepts an order id as an alternative to an Order object" do
Spree::Order.should_receive(:find).with(order.id).and_return(order)
lambda {
cancel_email = Spree::OrderMailer.cancel_email(order.id)
}.should_not raise_error
end

context "only shows eligible adjustments in emails" do
before do
order.adjustments.create({:label => "Eligible Adjustment",
Expand Down
7 changes: 7 additions & 0 deletions core/spec/mailers/shipment_mailer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,13 @@
shipment_email.body.should_not include(%Q{Out of Stock})
end

it "shipment_email accepts an shipment id as an alternative to an Shipment object" do
Spree::Shipment.should_receive(:find).with(shipment.id).and_return(shipment)
lambda {
shipped_email = Spree::ShipmentMailer.shipped_email(shipment.id)
}.should_not raise_error
end

context "emails must be translatable" do
context "shipped_email" do
context "en locale" do
Expand Down
7 changes: 6 additions & 1 deletion core/spec/models/spree/order/state_machine_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,14 @@
order.stub :has_available_shipment
order.stub :restock_items!
mail_message = mock "Mail::Message"
Spree::OrderMailer.should_receive(:cancel_email).with(order).and_return mail_message
order_id = nil
Spree::OrderMailer.should_receive(:cancel_email) { |*args|
order_id = args[0]
mail_message
}
mail_message.should_receive :deliver
order.cancel!
order_id.should == order.id
end

context "restocking inventory" do
Expand Down
4 changes: 2 additions & 2 deletions core/spec/models/spree/order_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -140,13 +140,13 @@ def compute(computable)

it "should send an order confirmation email" do
mail_message = mock "Mail::Message"
Spree::OrderMailer.should_receive(:confirm_email).with(order).and_return mail_message
Spree::OrderMailer.should_receive(:confirm_email).with(order.id).and_return mail_message
mail_message.should_receive :deliver
order.finalize!
end

it "should continue even if confirmation email delivery fails" do
Spree::OrderMailer.should_receive(:confirm_email).with(order).and_raise 'send failed!'
Spree::OrderMailer.should_receive(:confirm_email).with(order.id).and_raise 'send failed!'
order.finalize!
end

Expand Down
7 changes: 6 additions & 1 deletion core/spec/models/spree/shipment_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -319,9 +319,14 @@

it "should send a shipment email" do
mail_message = mock 'Mail::Message'
Spree::ShipmentMailer.should_receive(:shipped_email).with(shipment).and_return mail_message
shipment_id = nil
Spree::ShipmentMailer.should_receive(:shipped_email) { |*args|
shipment_id = args[0]
mail_message
}
mail_message.should_receive :deliver
shipment.ship!
shipment_id.should == shipment.id
end

it "should finalize the shipment's adjustment" do
Expand Down

0 comments on commit 8d90ae1

Please sign in to comment.