Merge pull request #2894 from rymai/9026-fix-n-plus-1-in-orders-contr…

…oller

Fix a N+1 query problem in the orders controller

backend/app/controllers/spree/admin/orders_controller.rb

@@ -51,17 +51,7 @@ def index
         end
 
         @search = Spree::Order.accessible_by(current_ability, :index).ransack(params[:q])
-
-        # lazy loading other models here (via includes) may result in an invalid query
-        # e.g. SELECT  DISTINCT DISTINCT "spree_orders".id, "spree_orders"."created_at" AS alias_0 FROM "spree_orders"
-        # see https://github.com/spree/spree/pull/3919
-        @orders = if query_present
-          @search.result(distinct: true)
-        else
-          @search.result
-        end
-
-        @orders = @orders.
+        @orders = @search.result.includes([:user]).
           page(params[:page]).
           per(params[:per_page] || Spree::Config[:orders_per_page])
 

backend/spec/controllers/spree/admin/orders_controller_spec.rb

@@ -69,7 +69,7 @@
       it "can page through the orders" do
         get :index, params: { page: 2, per_page: 10 }
         expect(assigns[:orders].offset_value).to eq(10)
-        expect(assigns[:orders].current_per_page).to eq(10)
+        expect(assigns[:orders].limit_value).to eq(10)
       end
     end
 
@@ -291,15 +291,27 @@
         allow(controller).to receive_messages spree_current_user: user
         user.spree_roles << Spree::Role.find_or_create_by(name: 'admin')
 
-        create(:completed_order_with_totals)
-        expect(Spree::Order.count).to eq 1
+        create_list(:completed_order_with_totals, 2)
+        expect(Spree::Order.count).to eq 2
       end
 
-      it "does not display duplicated results" do
-        get :index, params: { q: {
-          line_items_variant_id_in: Spree::Order.first.variants.map(&:id)
-        } }
-        expect(assigns[:orders].map(&:number).count).to eq 1
+      context 'by line_items_variant_id_in' do
+        it "does not display duplicated results" do
+          get :index, params: { q: {
+            line_items_variant_id_in: Spree::Order.first.variants.map(&:id)
+            } }
+          expect(assigns[:orders].size).to eq 1
+        end
+      end
+
+      context 'by email' do
+        it "does not display duplicated results" do
+          get :index, params: { q: {
+            email_start: Spree::Order.first.email
+            } }
+          expect(assigns[:orders].size).to eq 1
+          expect(assigns[:orders][0].email).to eq(Spree::Order.first.email)
+        end
       end
     end