Split ConfirmOrderJob to avoid blocking a worker

This unties this two email notifications so that they are picked up by
a DJ worker independently. This should avoid the blocking the worker experiences (remember we
still have a single one in all instances) when waiting between the two
deliveries. See the flamegraph:
https://app.datadoghq.com/apm/traces?end=1622015605459&paused=true&query=env%3Aproduction%20service%3Adelayed_job%20operation_name%3Adelayed_job%20resource_name%3AConfirmOrderJob%20%40duration%3A%3E%3D5s&start=1622009898303&streamTraces=true&trace=AQAAAXmngbg_woqc_QAAAABBWG1uZ2IwVkFBRHVDbWJkN25QTUVuY28&traceID=2916038355421570548&spanID=2005781139590273685.

Overall, both operations may take longer but other jobs can be processed
in between. Also, if any of the two fails, the other won't be affected.
This commit is contained in:
Pau Perez
2021-05-27 12:55:52 +02:00
parent 7f9179193e
commit 74a8b3038a
7 changed files with 31 additions and 42 deletions

View File

@@ -1,8 +0,0 @@
# frozen_string_literal: true
class ConfirmOrderJob < ActiveJob::Base
def perform(order_id)
Spree::OrderMailer.confirm_email_for_customer(order_id).deliver_now
Spree::OrderMailer.confirm_email_for_shop(order_id).deliver_now
end
end

View File

@@ -395,7 +395,8 @@ module Spree
def deliver_order_confirmation_email
return if subscription.present?
ConfirmOrderJob.perform_later(id)
Spree::OrderMailer.confirm_email_for_customer(id).deliver_later
Spree::OrderMailer.confirm_email_for_shop(id).deliver_later
end
# Helper methods for checkout steps

View File

@@ -251,10 +251,14 @@ feature "As a consumer I want to check out my cart", js: true do
choose shipping_with_fee.name
choose check_without_fee.name
expect do
perform_enqueued_jobs do
place_order
expect(page).to have_content "Your order has been processed successfully"
end.to enqueue_job ConfirmOrderJob
expect(ActionMailer::Base.deliveries.first.subject).to match(/Order Confirmation/)
expect(ActionMailer::Base.deliveries.second.subject).to match(/Order Confirmation/)
end
order = Spree::Order.complete.last
expect(order.payment_state).to eq "balance_due"
@@ -390,7 +394,7 @@ feature "As a consumer I want to check out my cart", js: true do
expect do
place_order
expect(page).to have_content "Your order has been processed successfully"
end.to enqueue_job ConfirmOrderJob
end.to have_enqueued_mail(Spree::OrderMailer, :confirm_email_for_customer)
# And the order's special instructions should be set
order = Spree::Order.complete.last

View File

@@ -1,18 +0,0 @@
# frozen_string_literal: true
require 'spec_helper'
describe ConfirmOrderJob do
let(:order) { create(:order) }
it "sends confirmation emails to both the user and the shop owner" do
customer_confirm_fake = double(:confirm_email_for_customer)
shop_confirm_fake = double(:confirm_email_for_shop)
expect(Spree::OrderMailer).to receive(:confirm_email_for_customer).and_return customer_confirm_fake
expect(Spree::OrderMailer).to receive(:confirm_email_for_shop).and_return shop_confirm_fake
expect(customer_confirm_fake).to receive :deliver_now
expect(shop_confirm_fake).to receive :deliver_now
ConfirmOrderJob.perform_now order.id
end
end

View File

@@ -237,7 +237,9 @@ describe SubscriptionConfirmJob do
end
it "sends only a subscription confirm email, no regular confirmation emails" do
expect{ job.send(:confirm_order!, order) }.to_not enqueue_job ConfirmOrderJob
expect{ job.send(:confirm_order!, order) }
.to_not have_enqueued_mail(Spree::OrderMailer, :confirm_email_for_customer)
expect(job).to have_received(:send_confirmation_email).once
end
end

View File

@@ -202,7 +202,9 @@ describe SubscriptionPlacementJob do
end
it "does not enqueue confirmation emails" do
expect{ job.send(:place_order, order) }.to_not enqueue_job ConfirmOrderJob
expect{ job.send(:place_order, order) }
.to_not have_enqueued_mail(Spree::OrderMailer, :confirm_email_for_customer)
expect(job).to have_received(:send_placement_email).with(order, anything).once
end

View File

@@ -169,10 +169,13 @@ describe Spree::Order do
expect(order.shipment_state).to eq 'ready'
end
it "should send an order confirmation email" do
expect do
order.finalize!
end.to enqueue_job ConfirmOrderJob
it "sends confirmation emails to both the user and the shop owner" do
mailer = double(:mailer, deliver_later: true)
expect(Spree::OrderMailer).to receive(:confirm_email_for_customer).and_return(mailer)
expect(Spree::OrderMailer).to receive(:confirm_email_for_shop).and_return(mailer)
order.finalize!
end
it "should freeze all adjustments" do
@@ -901,17 +904,20 @@ describe Spree::Order do
let!(:order) { create(:order, distributor: distributor) }
it "sends confirmation emails" do
expect do
order.deliver_order_confirmation_email
end.to enqueue_job ConfirmOrderJob
mailer = double(:mailer, deliver_later: true)
expect(Spree::OrderMailer).to receive(:confirm_email_for_customer).and_return(mailer)
expect(Spree::OrderMailer).to receive(:confirm_email_for_shop).and_return(mailer)
order.deliver_order_confirmation_email
end
it "does not send confirmation emails when the order belongs to a subscription" do
create(:proxy_order, order: order)
expect do
order.deliver_order_confirmation_email
end.to_not enqueue_job ConfirmOrderJob
expect(Spree::OrderMailer).not_to receive(:confirm_email_for_customer)
expect(Spree::OrderMailer).not_to receive(:confirm_email_for_shop)
order.deliver_order_confirmation_email
end
end