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 #2806
  • Loading branch information
petergoldstein authored and radar committed Apr 10, 2013
1 parent 83d2480 commit 88c00e1
Show file tree
Hide file tree
Showing 10 changed files with 53 additions and 17 deletions.
2 changes: 1 addition & 1 deletion core/app/controllers/spree/admin/orders_controller.rb
Expand Up @@ -102,7 +102,7 @@ def fire
end

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

respond_with(@order) { |format| format.html { redirect_to :back } }
Expand Down
17 changes: 11 additions & 6 deletions core/app/mailers/spree/order_mailer.rb
Expand Up @@ -7,21 +7,26 @@ def from_address
end

def confirm_email(order, resend = false)
@order = order
find_order(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
find_order(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

def find_order(order)
@order = order.is_a?(Spree::Order) ? order : Spree::Order.find(order)
end

end
end
6 changes: 3 additions & 3 deletions core/app/mailers/spree/shipment_mailer.rb
Expand Up @@ -7,10 +7,10 @@ def from_address
end

def shipped_email(shipment, resend = false)
@shipment = shipment
@shipment = shipment.is_a?(Spree::Shipment) ? 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
Expand Up @@ -388,7 +388,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 @@ -637,7 +637,7 @@ def after_cancel
restock_items!

#TODO: make_shipments_pending
OrderMailer.cancel_email(self).deliver
OrderMailer.cancel_email(self.id).deliver
unless %w(partial shipped).include?(shipment_state)
self.payment_state = 'credit_owed'
end
Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/shipment.rb
Expand Up @@ -144,7 +144,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
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
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/order/state_machine_spec.rb
Expand Up @@ -132,9 +132,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/order_spec.rb
Expand Up @@ -122,13 +122,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/shipment_spec.rb
Expand Up @@ -158,9 +158,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
end

Expand Down

0 comments on commit 88c00e1

Please sign in to comment.