From 0ec35b1f0d5d387bffd7c0fec0f32ef489e07cb3 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Tue, 7 Sep 2021 16:32:57 +0200 Subject: [PATCH 1/5] Only shows order that actually have at least one line_item Filter inside the API to shows only orders that have at least one line items --- app/models/spree/order.rb | 4 ++++ app/services/search_orders.rb | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index ef568bca36..de7da9497e 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -106,6 +106,10 @@ module Spree before_save :update_payment_fees!, if: :complete? # -- Scopes + scope :not_empty, -> { + joins(:line_items).group(:id).having("count(spree_line_items.id) > 0") + } + scope :managed_by, lambda { |user| if user.has_spree_role?('admin') where(nil) diff --git a/app/services/search_orders.rb b/app/services/search_orders.rb index 0da52721f3..4b2f87e396 100644 --- a/app/services/search_orders.rb +++ b/app/services/search_orders.rb @@ -23,7 +23,8 @@ class SearchOrders end def search_query - base_query = ::Permissions::Order.new(current_user).editable_orders + base_query = ::Permissions::Order.new(current_user).editable_orders.not_empty + return base_query unless params[:shipping_method_id] base_query From 9c86adf0f42062f2feeed9769585eeb959a14268 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Wed, 8 Sep 2021 09:41:19 +0200 Subject: [PATCH 2/5] Add line_items to each orders Now, API returns only orders with line_items: add to each in specs. --- spec/features/admin/orders_spec.rb | 9 +++++---- spec/requests/api/orders_spec.rb | 7 +++++-- spec/services/search_orders_spec.rb | 8 ++++---- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/spec/features/admin/orders_spec.rb b/spec/features/admin/orders_spec.rb index fa4757a424..a27a18c522 100644 --- a/spec/features/admin/orders_spec.rb +++ b/spec/features/admin/orders_spec.rb @@ -119,7 +119,7 @@ feature ' context "with incomplete order" do scenario "can edit order" do - incomplete_order = create(:order, distributor: distributor, order_cycle: order_cycle) + incomplete_order = create(:order_with_line_items, distributor: distributor, order_cycle: order_cycle, line_items_count: 1) login_as_admin_and_visit spree.admin_orders_path uncheck 'Only show complete orders' @@ -133,15 +133,16 @@ feature ' context "test the 'Only show the complete orders' checkbox" do scenario "display or not incomplete order" do - incomplete_order = create(:order, distributor: distributor, order_cycle: order_cycle) + incomplete_order = create(:order_with_line_items, distributor: distributor, order_cycle: order_cycle, line_items_count: 1) complete_order = create( - :order, + :order_with_line_items, distributor: distributor, order_cycle: order_cycle, user: user, state: 'complete', payment_state: 'balance_due', - completed_at: 1.day.ago + completed_at: 1.day.ago, + line_items_count: 1 ) login_as_admin_and_visit spree.admin_orders_path expect(page).to have_content complete_order.number diff --git a/spec/requests/api/orders_spec.rb b/spec/requests/api/orders_spec.rb index ef25152bbc..46bc44c38a 100644 --- a/spec/requests/api/orders_spec.rb +++ b/spec/requests/api/orders_spec.rb @@ -38,15 +38,18 @@ describe 'api/v0/orders', type: :request do let!(:order_dist_1) { create(:order_with_distributor, email: "specific_name@example.com") } + let!(:li1) { create(:line_item, order: order_dist_1) } let!(:order_dist_2) { create(:order_with_totals_and_distribution) } + let!(:li2) { create(:line_item, order: order_dist_2) } let!(:order_dist_1_complete) { - create(:order, distributor: order_dist_1.distributor, state: 'complete', - completed_at: Time.zone.today - 7.days) + create(:completed_order_with_totals, distributor: order_dist_1.distributor, state: 'complete', + completed_at: Time.zone.today - 7.days, line_items_count: 1) } let!(:order_dist_1_credit_owed) { create(:order, distributor: order_dist_1.distributor, payment_state: 'credit_owed', completed_at: Time.zone.today) } + let!(:li4) { create(:line_item_with_shipment, order: order_dist_1_credit_owed) } let(:user) { order_dist_1.distributor.owner } let(:'X-Spree-Token') do diff --git a/spec/services/search_orders_spec.rb b/spec/services/search_orders_spec.rb index 145e09e33f..73496ac123 100644 --- a/spec/services/search_orders_spec.rb +++ b/spec/services/search_orders_spec.rb @@ -4,9 +4,9 @@ require 'spec_helper' describe SearchOrders do let!(:distributor) { create(:distributor_enterprise) } - let!(:order1) { create(:order, distributor: distributor) } - let!(:order2) { create(:order, distributor: distributor) } - let!(:order3) { create(:order, distributor: distributor) } + let!(:order1) { create(:order_with_line_items, distributor: distributor, line_items_count: 3) } + let!(:order2) { create(:order_with_line_items, distributor: distributor, line_items_count: 2) } + let!(:order3) { create(:order_with_line_items, distributor: distributor, line_items_count: 1) } let(:enterprise_user) { distributor.owner } @@ -15,7 +15,7 @@ describe SearchOrders do let(:service) { SearchOrders.new(params, enterprise_user) } it 'returns orders' do - expect(service.orders.count).to eq 3 + expect(service.orders.count.length).to eq 3 end end end From 7656f5d20fb345c61f06854bc4fe09e8c4593ed4 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Wed, 8 Sep 2021 09:41:37 +0200 Subject: [PATCH 3/5] Add an order with no line_items, should not be returned by API --- spec/features/admin/orders_spec.rb | 3 +++ spec/requests/api/orders_spec.rb | 4 ++++ spec/services/search_orders_spec.rb | 1 + 3 files changed, 8 insertions(+) diff --git a/spec/features/admin/orders_spec.rb b/spec/features/admin/orders_spec.rb index a27a18c522..6b724b02f3 100644 --- a/spec/features/admin/orders_spec.rb +++ b/spec/features/admin/orders_spec.rb @@ -144,15 +144,18 @@ feature ' completed_at: 1.day.ago, line_items_count: 1 ) + empty_order = create(:order, distributor: distributor, order_cycle: order_cycle) login_as_admin_and_visit spree.admin_orders_path expect(page).to have_content complete_order.number expect(page).to have_no_content incomplete_order.number + expect(page).to have_no_content empty_order.number uncheck 'Only show complete orders' page.find('a.icon-search').click expect(page).to have_content complete_order.number expect(page).to have_content incomplete_order.number + expect(page).to have_no_content empty_order.number end end diff --git a/spec/requests/api/orders_spec.rb b/spec/requests/api/orders_spec.rb index 46bc44c38a..36c38b1b0e 100644 --- a/spec/requests/api/orders_spec.rb +++ b/spec/requests/api/orders_spec.rb @@ -51,6 +51,10 @@ describe 'api/v0/orders', type: :request do } let!(:li4) { create(:line_item_with_shipment, order: order_dist_1_credit_owed) } + let!(:order_empty) { + create(:order_with_line_items, line_items_count: 0) + } + let(:user) { order_dist_1.distributor.owner } let(:'X-Spree-Token') do user.generate_spree_api_key! diff --git a/spec/services/search_orders_spec.rb b/spec/services/search_orders_spec.rb index 73496ac123..8a51c5295a 100644 --- a/spec/services/search_orders_spec.rb +++ b/spec/services/search_orders_spec.rb @@ -7,6 +7,7 @@ describe SearchOrders do let!(:order1) { create(:order_with_line_items, distributor: distributor, line_items_count: 3) } let!(:order2) { create(:order_with_line_items, distributor: distributor, line_items_count: 2) } let!(:order3) { create(:order_with_line_items, distributor: distributor, line_items_count: 1) } + let!(:order_empty) { create(:order, distributor: distributor) } let(:enterprise_user) { distributor.owner } From b53371d870260d983f3ed3dc48861e390717ea46 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Wed, 8 Sep 2021 12:14:53 +0100 Subject: [PATCH 4/5] Fix not_empty scope --- app/models/spree/order.rb | 2 +- spec/services/search_orders_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index de7da9497e..b2fa712d00 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -107,7 +107,7 @@ module Spree # -- Scopes scope :not_empty, -> { - joins(:line_items).group(:id).having("count(spree_line_items.id) > 0") + left_outer_joins(:line_items).where.not(spree_line_items: { id: nil }) } scope :managed_by, lambda { |user| diff --git a/spec/services/search_orders_spec.rb b/spec/services/search_orders_spec.rb index 8a51c5295a..e4b543aacd 100644 --- a/spec/services/search_orders_spec.rb +++ b/spec/services/search_orders_spec.rb @@ -16,7 +16,7 @@ describe SearchOrders do let(:service) { SearchOrders.new(params, enterprise_user) } it 'returns orders' do - expect(service.orders.count.length).to eq 3 + expect(service.orders.count).to eq 3 end end end From 2d2e0a5b24b6146b842b40b8f2f72a4636fd4728 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Wed, 8 Sep 2021 12:18:08 +0100 Subject: [PATCH 5/5] Add unit test for not_empty scope --- spec/models/spree/order_spec.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index e6b7086e98..defe01ac29 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -849,6 +849,16 @@ describe Spree::Order do expect(Spree::Order.not_state(:canceled)).not_to include o end end + + describe "not_empty" do + let!(:order_with_line_items) { create(:order_with_line_items, line_items_count: 1) } + let!(:order_without_line_items) { create(:order) } + + it "returns only orders which have line items" do + expect(Spree::Order.not_empty).to include order_with_line_items + expect(Spree::Order.not_empty).to_not include order_without_line_items + end + end end describe "sending confirmation emails" do