Merge pull request #4417 from Matt-Yorkley/shopfront_patchination

Ensure variants returned in #variant_relation are DISTINCT

app/services/order_cycle_distributed_products.rb

@@ -8,13 +8,14 @@ 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
     order_cycle.
       variants_distributed_by(distributor).
-      merge(stocked_variants_and_overrides)
+      merge(stocked_variants_and_overrides).
+      select("DISTINCT spree_variants.*")
   end
 
   private

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, spree_products.id ASC"
     else
-      "name ASC"
+      "spree_products.name ASC"
     end
   end
 

spec/controllers/api/order_cycles_controller_spec.rb

@@ -187,6 +187,38 @@ 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).to eq [product7.id, product8.id, product2.id, product3.id, product5.id, product6.id, product1.id]
+      end
+
+      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).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).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).to eq [product1.id]
+      end
+    end
+
     private
 
     def product_ids