Make order.order_cycle required in cart_service. There’s no more exception for that without product_distributions.

app/services/cart_service.rb

@@ -127,11 +127,11 @@ class CartService
   end
 
   def valid_variant?(variant)
-    check_order_cycle_provided_for(variant) && check_variant_available_under_distribution(variant)
+    check_order_cycle_provided && check_variant_available_under_distribution(variant)
   end
 
-  def check_order_cycle_provided_for(variant)
-    order_cycle_provided = (!order_cycle_required_for(variant) || @order_cycle.present?)
+  def check_order_cycle_provided
+    order_cycle_provided = @order_cycle.present?
     errors.add(:base, "Please choose an order cycle for this order.") unless order_cycle_provided
     order_cycle_provided
   end
@@ -143,10 +143,6 @@ class CartService
     false
   end
 
-  def order_cycle_required_for(variant)
-    variant.product.product_distributions.empty?
-  end
-
   def line_item_for_variant_id(variant_id)
     order.find_line_item_by_variant Spree::Variant.find(variant_id)
   end

lib/open_food_network/distribution_change_validator.rb

@@ -9,11 +9,7 @@ class DistributionChangeValidator
   end
 
   def variants_available_for_distribution(distributor, order_cycle)
-    product_distribution_variants = []
-    product_distribution_variants = distributor.product_distribution_variants if distributor
-    order_cycle_variants = []
-    order_cycle_variants = order_cycle.variants_distributed_by(distributor) if order_cycle
-
-    product_distribution_variants + order_cycle_variants
+    return [] unless order_cycle
+    order_cycle.variants_distributed_by(distributor)
   end
 end

spec/controllers/cart_controller_spec.rb

@@ -90,16 +90,19 @@ describe CartController, type: :controller do
 
   context "adding a group buy product to the cart" do
     it "sets a variant attribute for the max quantity" do
-      distributor_product = create(:distributor_enterprise)
-      p = create(:product, distributors: [distributor_product], group_buy: true)
+      distributor = create(:distributor_enterprise)
+      product = create(:product, group_buy: true)
+      variant = product.variants.first
+      order_cycle = create(:simple_order_cycle, distributors: [distributor], variants: [variant])
 
       order = subject.current_order(true)
-      allow(order).to receive(:distributor) { distributor_product }
-      expect(order).to receive(:set_variant_attributes).with(p.master, max_quantity: '3')
+      allow(order).to receive(:distributor) { distributor }
+      allow(order).to receive(:order_cycle) { order_cycle }
+      expect(order).to receive(:set_variant_attributes).with(variant, max_quantity: '3')
       allow(controller).to receive(:current_order).and_return(order)
 
       expect do
-        spree_post :populate, variants: { p.master.id => 1 }, variant_attributes: { p.master.id => { max_quantity: 3 } }
+        spree_post :populate, variants: { variant.id => 1 }, variant_attributes: { variant.id => { max_quantity: 3 } }
       end.to change(Spree::LineItem, :count).by(1)
     end
   end

spec/lib/open_food_network/distribution_change_validator_spec.rb

@@ -24,25 +24,10 @@ describe DistributionChangeValidator do
     end
   end
 
-  describe "finding variants that are available through a particular distribution" do
-    it "finds variants distributed by product distribution" do
-      variant = double(:variant)
-      distributor = double(:distributor, product_distribution_variants: [variant])
-      order_cycle = double(:order_cycle, variants_distributed_by: [])
-
-      expect(subject.variants_available_for_distribution(distributor, order_cycle)).to eq [variant]
-    end
-
-    it "finds variants distributed by product distribution when order cycle is nil" do
-      variant = double(:variant)
-      distributor = double(:distributor, product_distribution_variants: [variant])
-
-      expect(subject.variants_available_for_distribution(distributor, nil)).to eq [variant]
-    end
-
+  describe "finding variants that are available through a particular order cycle" do
     it "finds variants distributed by order cycle" do
       variant = double(:variant)
-      distributor = double(:distributor, product_distribution_variants: [])
+      distributor = double(:distributor)
       order_cycle = double(:order_cycle)
 
       order_cycle.should_receive(:variants_distributed_by).with(distributor) { [variant] }
@@ -50,7 +35,7 @@ describe DistributionChangeValidator do
       expect(subject.variants_available_for_distribution(distributor, order_cycle)).to eq [variant]
     end
 
-    it "returns an empty array when distributor and order cycle are both nil" do
+    it "returns an empty array when order cycle is nil" do
       expect(subject.variants_available_for_distribution(nil, nil)).to eq []
     end
   end

spec/services/cart_service_spec.rb

@@ -130,7 +130,7 @@ describe CartService do
     end
 
     it "performs additional validations" do
-      expect(cart_service).to receive(:check_order_cycle_provided_for).with(variant).and_return(true)
+      expect(cart_service).to receive(:check_order_cycle_provided) { true }
       expect(cart_service).to receive(:check_variant_available_under_distribution).with(variant).
         and_return(true)
       expect(order).to receive(:add_variant).with(variant, quantity, nil, currency)
@@ -142,7 +142,7 @@ describe CartService do
       expect(cart_service).to receive(:quantities_to_add).with(variant, 123, 123).
         and_return([5, 5])
 
-      allow(cart_service).to receive(:check_order_cycle_provided_for) { true }
+      allow(cart_service).to receive(:check_order_cycle_provided) { true }
       allow(cart_service).to receive(:check_variant_available_under_distribution) { true }
 
       expect(order).to receive(:add_variant).with(variant, 5, 5, currency)
@@ -154,7 +154,7 @@ describe CartService do
       expect(cart_service).to receive(:quantities_to_add).with(variant, 123, 123).
         and_return([0, 0])
 
-      allow(cart_service).to receive(:check_order_cycle_provided_for) { true }
+      allow(cart_service).to receive(:check_order_cycle_provided) { true }
       allow(cart_service).to receive(:check_variant_available_under_distribution) { true }
 
       expect(order).to receive(:remove_variant).with(variant)
@@ -208,19 +208,14 @@ describe CartService do
     describe "checking order cycle is provided for a variant, OR is not needed" do
       let(:variant) { double(:variant) }
 
-      it "returns false and errors when order cycle is not provided and is required" do
-        allow(cart_service).to receive(:order_cycle_required_for).and_return true
-        expect(cart_service.send(:check_order_cycle_provided_for, variant)).to be false
+      it "returns false and errors when order cycle is not provided" do
+        expect(cart_service.send(:check_order_cycle_provided)).to be false
         expect(cart_service.errors.to_a).to eq(["Please choose an order cycle for this order."])
       end
+
       it "returns true when order cycle is provided" do
-        allow(cart_service).to receive(:order_cycle_required_for).and_return true
         cart_service.instance_variable_set :@order_cycle,  double(:order_cycle)
-        expect(cart_service.send(:check_order_cycle_provided_for, variant)).to be true
-      end
-      it "returns true when order cycle is not required" do
-        allow(cart_service).to receive(:order_cycle_required_for).and_return false
-        expect(cart_service.send(:check_order_cycle_provided_for, variant)).to be true
+        expect(cart_service.send(:check_order_cycle_provided)).to be true
       end
     end
 
@@ -247,26 +242,4 @@ describe CartService do
       end
     end
   end
-
-  describe "support" do
-    describe "checking if order cycle is required for a variant" do
-      it "requires an order cycle when the product has no product distributions" do
-        product = double(:product, product_distributions: [])
-        variant = double(:variant, product: product)
-        expect(cart_service.send(:order_cycle_required_for, variant)).to be true
-      end
-
-      it "does not require an order cycle when the product has product distributions" do
-        product = double(:product, product_distributions: [1])
-        variant = double(:variant, product: product)
-        expect(cart_service.send(:order_cycle_required_for, variant)).to be false
-      end
-    end
-
-    it "provides the distributor and order cycle for the order" do
-      expect(order).to receive(:distributor).and_return(distributor)
-      expect(order).to receive(:order_cycle).and_return(order_cycle)
-      expect(cart_service.send(:distributor_and_order_cycle)).to eq([distributor, order_cycle])
-    end
-  end
 end