mirror of
https://github.com/openfoodfoundation/openfoodnetwork
synced 2026-03-01 02:03:22 +00:00
Eager load payment and subs. order associations
This removes the N+1 queries caused by `Api::Admin::OrderSerialier#ready_to_capture` when used from `Api::OrdersController#index`. While it's fine for the single-order controller actions, it's not for this one that deals with a collection of orders. Fortunately, `SearchOrders` is used only in this controller action so we can put the `includes` calls there, otherwise, we would need to refactor it a bit to pass in a context-specific AR relation.
This commit is contained in:
@@ -38,9 +38,11 @@ module Api
|
||||
spree_routes_helper.admin_order_payments_path(object)
|
||||
end
|
||||
|
||||
# This methods requires to eager load the payment association (with its required WHERE
|
||||
# constraints) so as not to cause and N+1.
|
||||
def ready_to_capture
|
||||
pending_payment = object.pending_payments.reject(&:authorization_action_required?).first
|
||||
object.payment_required? && pending_payment.present?
|
||||
pending_payments = object.pending_payments.reject(&:authorization_action_required?)
|
||||
object.payment_required? && pending_payments.any?
|
||||
end
|
||||
|
||||
def ready_to_ship
|
||||
|
||||
@@ -13,7 +13,9 @@ class SearchOrders
|
||||
attr_reader :params, :current_user
|
||||
|
||||
def fetch_orders
|
||||
@search = search_query.ransack(params[:q])
|
||||
@search = search_query.
|
||||
includes(:payments, :subscription).
|
||||
ransack(params[:q])
|
||||
|
||||
return paginated_results if using_pagination?
|
||||
|
||||
|
||||
@@ -157,6 +157,47 @@ module Api
|
||||
expect(json_response['pagination']).to eq pagination_data
|
||||
end
|
||||
end
|
||||
|
||||
context "when there is a pending payment requiring authorization" do
|
||||
let!(:pending_payment) do
|
||||
create(
|
||||
:payment,
|
||||
order: order1,
|
||||
state: 'pending',
|
||||
amount: 123.45,
|
||||
cvv_response_message: "https://stripe.com/redirect"
|
||||
)
|
||||
end
|
||||
|
||||
before do
|
||||
allow(controller).to receive(:spree_current_user) { distributor.owner }
|
||||
end
|
||||
|
||||
it "returns false" do
|
||||
get :index
|
||||
expect(json_response['orders'].first['ready_to_capture']).to eq(false)
|
||||
end
|
||||
end
|
||||
|
||||
context "when there is a pending payment but it does not require authorization" do
|
||||
let!(:pending_payment) do
|
||||
create(
|
||||
:payment,
|
||||
order: order1,
|
||||
state: 'pending',
|
||||
amount: 123.45,
|
||||
)
|
||||
end
|
||||
|
||||
before do
|
||||
allow(controller).to receive(:spree_current_user) { distributor.owner }
|
||||
end
|
||||
|
||||
it "returns true" do
|
||||
get :index
|
||||
expect(json_response['orders'].first['ready_to_capture']).to eq(true)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe "#show" do
|
||||
|
||||
@@ -36,8 +36,8 @@ describe Api::Admin::OrderSerializer do
|
||||
allow(order).to receive(:payment_required?) { true }
|
||||
end
|
||||
|
||||
context "there is a payment pending authorization" do
|
||||
let!(:pending_payment) {
|
||||
context "there is a pending payment requiring authorization" do
|
||||
let!(:pending_payment) do
|
||||
create(
|
||||
:payment,
|
||||
order: order,
|
||||
@@ -45,24 +45,24 @@ describe Api::Admin::OrderSerializer do
|
||||
amount: 123.45,
|
||||
cvv_response_message: "https://stripe.com/redirect"
|
||||
)
|
||||
}
|
||||
end
|
||||
|
||||
it "returns false if there is a payment requiring authorization" do
|
||||
it "returns false" do
|
||||
expect(serializer.ready_to_capture).to be false
|
||||
end
|
||||
end
|
||||
|
||||
context "there is a pending payment but it does not require authorization" do
|
||||
let!(:pending_payment) {
|
||||
let!(:pending_payment) do
|
||||
create(
|
||||
:payment,
|
||||
order: order,
|
||||
state: 'pending',
|
||||
amount: 123.45,
|
||||
)
|
||||
}
|
||||
end
|
||||
|
||||
it "returns true if there is no payment requiring authorization" do
|
||||
it "returns true" do
|
||||
expect(serializer.ready_to_capture).to be true
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user