Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change code to invoke mailers with ids #2808

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion backend/app/controllers/spree/admin/orders_controller.rb
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
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
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
Expand Up @@ -390,7 +390,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 @@ -548,7 +548,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
Expand Up @@ -258,7 +258,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 @@ -29,6 +29,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/spree/order/state_machine_spec.rb
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|
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't know you could do this. Neat.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I like this trick. This technique is especially handy when an object is being created in the process of the test and it's not easy to mock the creation method.

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
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
Expand Up @@ -245,9 +245,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