From 6d50176e6bf48df064118410b8b2fdc84ae793f8 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Wed, 30 Oct 2019 13:46:52 +0000 Subject: [PATCH 1/4] Ensure results in #variants_relation are DISTINCT --- app/services/order_cycle_distributed_products.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/services/order_cycle_distributed_products.rb b/app/services/order_cycle_distributed_products.rb index 306233ea3c..12aab241c7 100644 --- a/app/services/order_cycle_distributed_products.rb +++ b/app/services/order_cycle_distributed_products.rb @@ -15,7 +15,8 @@ class OrderCycleDistributedProducts def variants_relation order_cycle. variants_distributed_by(distributor). - merge(stocked_variants_and_overrides) + merge(stocked_variants_and_overrides). + select("DISTINCT spree_variants.*") end private From f8209ac7d57af314e7febc8b87848ea740e03481 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Wed, 30 Oct 2019 17:14:20 +0000 Subject: [PATCH 2/4] Ensure results in #products_relation are DISTINCT --- app/services/order_cycle_distributed_products.rb | 2 +- app/services/products_renderer.rb | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/services/order_cycle_distributed_products.rb b/app/services/order_cycle_distributed_products.rb index 12aab241c7..f1b246f2dd 100644 --- a/app/services/order_cycle_distributed_products.rb +++ b/app/services/order_cycle_distributed_products.rb @@ -9,7 +9,7 @@ class OrderCycleDistributedProducts end def products_relation - Spree::Product.where(id: stocked_products) + Spree::Product.where(id: stocked_products).group("spree_products.id") end def variants_relation diff --git a/app/services/products_renderer.rb b/app/services/products_renderer.rb index 2d64ed4471..6e279a828a 100644 --- a/app/services/products_renderer.rb +++ b/app/services/products_renderer.rb @@ -63,10 +63,10 @@ class ProductsRenderer if distributor.preferred_shopfront_taxon_order.present? distributor .preferred_shopfront_taxon_order - .split(",").map { |id| "primary_taxon_id=#{id} DESC" } - .join(",") + ", name ASC" + .split(",").map { |id| "spree_products.primary_taxon_id=#{id} DESC" } + .join(", ") + ", spree_products.name ASC" else - "name ASC" + "spree_products.name ASC" end end From 9723e2cd4905a6a0b72031a6ee4797716c218b63 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 31 Oct 2019 12:07:16 +0000 Subject: [PATCH 3/4] Add failing spec for taxon ordering issue --- .../api/order_cycles_controller_spec.rb | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/spec/controllers/api/order_cycles_controller_spec.rb b/spec/controllers/api/order_cycles_controller_spec.rb index 1cf346277d..f24e3936cb 100644 --- a/spec/controllers/api/order_cycles_controller_spec.rb +++ b/spec/controllers/api/order_cycles_controller_spec.rb @@ -187,6 +187,46 @@ module Api end end + context "with custom taxon ordering applied and duplicate product names in the order cycle" do + let!(:supplier) { create(:supplier_enterprise) } + let!(:product5) { create(:product, name: "Duplicate name", primary_taxon: taxon3, supplier: supplier) } + let!(:product6) { create(:product, name: "Duplicate name", primary_taxon: taxon3, supplier: supplier) } + let!(:product7) { create(:product, name: "Duplicate name", primary_taxon: taxon2, supplier: supplier) } + let!(:product8) { create(:product, name: "Duplicate name", primary_taxon: taxon2, supplier: supplier) } + + before do + distributor.preferred_shopfront_taxon_order = "#{taxon2.id},#{taxon3.id},#{taxon1.id}" + exchange.variants << product5.variants.first + exchange.variants << product6.variants.first + exchange.variants << product7.variants.first + exchange.variants << product8.variants.first + end + + it "displays products in new order" do + api_get :products, id: order_cycle.id, distributor: distributor.id + + expect(product_ids.length).to eq 7 + expect(product_ids).to eq [product7.id, product8.id, product2.id, product3.id, product5.id, product6.id, product1.id] + end + + xit "displays products in correct order across multiple pages" do + api_get :products, id: order_cycle.id, distributor: distributor.id, per_page: 3 + + expect(product_ids.length).to eq 3 + expect(product_ids).to eq [product7.id, product8.id, product2.id] + + api_get :products, id: order_cycle.id, distributor: distributor.id, per_page: 3, page: 2 + + expect(product_ids.length).to eq 3 + expect(product_ids).to eq [product3.id, product5.id, product6.id] + + api_get :products, id: order_cycle.id, distributor: distributor.id, per_page: 3, page: 3 + + expect(product_ids.length).to eq 1 + expect(product_ids).to eq [product1.id] + end + end + private def product_ids From 67a5a1cdc2e809fce34ff097bbdc686f8606787d Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 31 Oct 2019 12:10:22 +0000 Subject: [PATCH 4/4] Fix incorrectly ordered entries with duplicate product names in OC and custom taxon ordering applied --- app/services/products_renderer.rb | 2 +- spec/controllers/api/order_cycles_controller_spec.rb | 10 +--------- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/app/services/products_renderer.rb b/app/services/products_renderer.rb index 6e279a828a..ca9c728267 100644 --- a/app/services/products_renderer.rb +++ b/app/services/products_renderer.rb @@ -64,7 +64,7 @@ class ProductsRenderer distributor .preferred_shopfront_taxon_order .split(",").map { |id| "spree_products.primary_taxon_id=#{id} DESC" } - .join(", ") + ", spree_products.name ASC" + .join(", ") + ", spree_products.name ASC, spree_products.id ASC" else "spree_products.name ASC" end diff --git a/spec/controllers/api/order_cycles_controller_spec.rb b/spec/controllers/api/order_cycles_controller_spec.rb index f24e3936cb..595e734113 100644 --- a/spec/controllers/api/order_cycles_controller_spec.rb +++ b/spec/controllers/api/order_cycles_controller_spec.rb @@ -204,25 +204,17 @@ module Api it "displays products in new order" do api_get :products, id: order_cycle.id, distributor: distributor.id - - expect(product_ids.length).to eq 7 expect(product_ids).to eq [product7.id, product8.id, product2.id, product3.id, product5.id, product6.id, product1.id] end - xit "displays products in correct order across multiple pages" do + it "displays products in correct order across multiple pages" do api_get :products, id: order_cycle.id, distributor: distributor.id, per_page: 3 - - expect(product_ids.length).to eq 3 expect(product_ids).to eq [product7.id, product8.id, product2.id] api_get :products, id: order_cycle.id, distributor: distributor.id, per_page: 3, page: 2 - - expect(product_ids.length).to eq 3 expect(product_ids).to eq [product3.id, product5.id, product6.id] api_get :products, id: order_cycle.id, distributor: distributor.id, per_page: 3, page: 3 - - expect(product_ids.length).to eq 1 expect(product_ids).to eq [product1.id] end end