From 46d997406a9bf07e626986d422024b7852b41b60 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Thu, 26 Nov 2020 09:59:27 +0100 Subject: [PATCH] Filter out cart orders when fetching customers We only care about non-cart orders and skipping carts, saves PostgreSQL query planner to go through thousands of records in production use cases (my food hub). We go from ```sql -> Index Scan using index_spree_orders_on_customer_id on spree_orders (cost=0.42..12049.45 rows=152002 width=15) (actual time=0.015..11.703 rows=13867 loops=1) ``` to ```sql -> Index Scan using index_spree_orders_on_customer_id on spree_orders (cost=0.42..12429.46 rows=10802 width=15) (actual time=0.025..17.705 rows=9954 loops=1) ``` --- app/services/customers_with_balance.rb | 9 +++++- .../admin/customers_controller_spec.rb | 4 +-- spec/features/admin/customers_spec.rb | 6 ++-- spec/services/customers_with_balance_spec.rb | 30 +++++++++++++++---- 4 files changed, 38 insertions(+), 11 deletions(-) diff --git a/app/services/customers_with_balance.rb b/app/services/customers_with_balance.rb index d64d9d5feb..3cf667882a 100644 --- a/app/services/customers_with_balance.rb +++ b/app/services/customers_with_balance.rb @@ -8,7 +8,7 @@ class CustomersWithBalance def query Customer.of(enterprise_id). includes(:bill_address, :ship_address, user: :credit_cards). - joins("LEFT JOIN spree_orders ON spree_orders.customer_id = customers.id"). + joins(left_join_non_cart_orders). group("customers.id"). select("customers.*"). select(outstanding_balance) @@ -29,4 +29,11 @@ class CustomersWithBalance ) AS balance_value SQL end + + def left_join_non_cart_orders + <<-SQL.strip_heredoc + LEFT JOIN spree_orders ON spree_orders.customer_id = customers.id + AND spree_orders.state != 'cart' + SQL + end end diff --git a/spec/controllers/admin/customers_controller_spec.rb b/spec/controllers/admin/customers_controller_spec.rb index 662f217f40..e33dd651ee 100644 --- a/spec/controllers/admin/customers_controller_spec.rb +++ b/spec/controllers/admin/customers_controller_spec.rb @@ -83,12 +83,12 @@ module Admin it 'includes the customer balance in the response' do spree_get :index, params - expect(json_response.first["balance"]).to eq("$-10.00") + expect(json_response.first["balance"]).to eq("$0.00") end end context 'when the customer has an order with a void payment' do - let(:order) { create(:order, customer: customer) } + let(:order) { create(:order, customer: customer, state: 'complete') } let!(:line_item) { create(:line_item, order: order, price: 10.0) } let!(:payment) { create(:payment, order: order, amount: order.total) } diff --git a/spec/features/admin/customers_spec.rb b/spec/features/admin/customers_spec.rb index 37b0595a2d..f0c1d2c6fa 100644 --- a/spec/features/admin/customers_spec.rb +++ b/spec/features/admin/customers_spec.rb @@ -103,7 +103,7 @@ feature 'Customers' do payment_total: 88, distributor: managed_distributor1, user: nil, - completed_at: Time.zone.now, + state: 'complete', customer: customer1 ) create( @@ -112,7 +112,7 @@ feature 'Customers' do payment_total: 0, distributor: managed_distributor1, user: nil, - completed_at: Time.zone.now, + state: 'complete', customer: customer2 ) create( @@ -121,7 +121,7 @@ feature 'Customers' do payment_total: 0, distributor: managed_distributor1, user: nil, - completed_at: Time.zone.now, + state: 'complete', customer: customer4 ) diff --git a/spec/services/customers_with_balance_spec.rb b/spec/services/customers_with_balance_spec.rb index 0edae9d1f0..0ad61fb8de 100644 --- a/spec/services/customers_with_balance_spec.rb +++ b/spec/services/customers_with_balance_spec.rb @@ -8,13 +8,30 @@ describe CustomersWithBalance do describe '#query' do let(:customer) { create(:customer) } + context 'when orders are in cart state' do + let(:total) { 200.00 } + let(:order_total) { 100.00 } + + before do + create(:order, customer: customer, total: order_total, payment_total: 0, state: 'cart') + create(:order, customer: customer, total: order_total, payment_total: 0, state: 'cart') + end + + it 'returns the customer balance' do + customer = customers_with_balance.query.first + expect(customer.balance_value).to eq(0) + end + end + context 'when no orders where paid' do let(:total) { 200.00 } let(:order_total) { 100.00 } before do - create(:order, customer: customer, total: order_total, payment_total: 0) - create(:order, customer: customer, total: order_total, payment_total: 0) + order = create(:order, customer: customer, total: order_total, payment_total: 0) + order.update_attribute(:state, 'checkout') + order = create(:order, customer: customer, total: order_total, payment_total: 0) + order.update_attribute(:state, 'checkout') end it 'returns the customer balance' do @@ -29,8 +46,10 @@ describe CustomersWithBalance do let(:payment_total) { order_total } before do - create(:order, customer: customer, total: order_total, payment_total: 0) - create(:order, customer: customer, total: order_total, payment_total: payment_total) + order = create(:order, customer: customer, total: order_total, payment_total: 0) + order.update_attribute(:state, 'checkout') + order = create(:order, customer: customer, total: order_total, payment_total: payment_total) + order.update_attribute(:state, 'checkout') end it 'returns the customer balance' do @@ -46,7 +65,8 @@ describe CustomersWithBalance do let(:non_canceled_orders_total) { order_total } before do - create(:order, customer: customer, total: order_total, payment_total: 0) + order = create(:order, customer: customer, total: order_total, payment_total: 0) + order.update_attribute(:state, 'checkout') create( :order, customer: customer,