Push validation logic for changing distributor down into lib class

app/controllers/spree/orders_controller_decorator.rb

@@ -34,11 +34,8 @@ Spree::OrdersController.class_eval do
     distributor = Enterprise.is_distributor.find params[:id]
 
     order = current_order(true)
-
-    if order.can_change_to_distributor?(distributor)
-      order.distributor = distributor
-      order.save!
-    end
+    order.distributor = distributor
+    order.save!
 
     redirect_to main_app.enterprise_path(distributor)
   end
@@ -46,10 +43,8 @@ Spree::OrdersController.class_eval do
   def deselect_distributor
     order = current_order(true)
 
-    if order.can_change_distributor?
-      order.distributor = nil
-      order.save!
-    end
+    order.distributor = nil
+    order.save!
 
     redirect_to root_path
   end
@@ -73,7 +68,7 @@ Spree::OrdersController.class_eval do
 
     # -- If products in cart, distributor can't be changed
     order = current_order(false)
-    if !order.nil? && !order.can_change_distributor? && order.distributor != distributor
+    if !order.nil? && !DistributorChangeValidator.new(order).can_change_to_distributor?(distributor) 
       return false
     end
 

app/models/enterprise.rb

@@ -27,6 +27,10 @@ class Enterprise < ActiveRecord::Base
   def to_param
     "#{id}-#{name.parameterize}"
   end
+  
+  def available_variants
+    ProductDistribution.find_all_by_distributor_id( self.id ).map{ |pd| pd.product.variants }.flatten
+  end
 
 
   private

app/models/spree/order_decorator.rb

@@ -1,44 +1,22 @@
+require 'open_food_web/distributor_change_validator'
+
 Spree::Order.class_eval do
   belongs_to :distributor, :class_name => 'Enterprise'
 
   before_validation :shipping_address_from_distributor
-  validate :change_distributor_validation, :if => :distributor_id_changed?
+  validate :products_available_from_new_distributor, :if => :distributor_id_changed?
   attr_accessible :distributor_id
 
   after_create :set_default_shipping_method
 
-  def change_distributor_validation
+  
+  def products_available_from_new_distributor
     # Check that the line_items in the current order are available from a newly selected distributor
-    errors.add(:distributor_id, "The products in your cart are not available from '" + distributor.name + "'") unless can_change_to_distributor? distributor
-  end
-
-  def can_change_to_distributor? distributor
-    # Distributor may not be changed once an item has been added to the cart/order, unless all items are available from the specified distributor
-    line_items.empty? || (available_distributors || []).include?(distributor)
-  end
-
-  def can_change_distributor?
-    # Distributor may not be changed once an item has been added to the cart/order
-    line_items.empty?
-  end
-
-  def available_distributors
-    # Find all other enterprises which offer all product variants contained within the current order
-    distributors_with_all_variants = get_distributors_with_all_variants(Enterprise.all)
-  end
-
-  def get_distributors_with_all_variants(enterprises)
-    variants_in_current_order = line_items.map{ |li| li.variant }
-    distributors_with_all_variants = []
-    enterprises.each do |e|
-      variants_available_from_enterprise = ProductDistribution.find_all_by_distributor_id( e.id ).map{ |pd| pd.product.variants }.flatten
-      distributors_with_all_variants << e if ( variants_in_current_order - variants_available_from_enterprise ).empty?
-    end
-    distributors_with_all_variants
+    errors.add(:distributor_id, "cannot supply the products in your cart") unless DistributorChangeValidator.new(self).can_change_to_distributor?(distributor)
   end
 
   def distributor=(distributor)
-    raise "You cannot change the distributor of an order with products" unless distributor == self.distributor || can_change_to_distributor?(distributor)
+    #raise "You cannot change the distributor of an order with products" unless distributor == self.distributor || DistributorChangeValidator.new(self).can_change_to_distributor?(distributor)
     super(distributor)
   end
 
@@ -47,11 +25,6 @@ Spree::Order.class_eval do
     save!
   end
 
-  def can_add_product_to_cart?(product)
-    # Products may be added if no line items are currently in the cart or if the product is available from the current distributor
-    line_items.empty? || product.distributors.include?(distributor)
-  end
-
   def set_variant_attributes(variant, attributes)
     line_item = contains?(variant)
 
@@ -62,6 +35,14 @@ Spree::Order.class_eval do
     line_item.assign_attributes(attributes)
     line_item.save!
   end
+  
+  def can_add_product_to_cart? product
+    DistributorChangeValidator.new(self).can_change_distributor? || product.distributors.include?(distributor)
+  end
+  
+  def line_item_variants
+    line_items.map{ |li| li.variant }
+  end
 
 
   private

app/views/spree/checkout/_other_available_distributors.html.erb

@@ -1,7 +1,7 @@
 <% unless @order.state != 'address' %>
 <div class="columns omega four" style="padding-top: 30px">
   <%
-    other_available_distributors = @order.available_distributors
+    other_available_distributors = DistributorChangeValidator.new(@order).available_distributors(Enterprise.all)
 	other_available_distributors.delete(@order.distributor)
     unless other_available_distributors.empty?
   %>

app/views/spree/products/_add_to_cart.html.haml

@@ -1,8 +1,9 @@
 .add-to-cart
+  - order = current_order(false)
   - if !@product.has_stock? && !Spree::Config[:allow_backorders]
     = content_tag('strong', t(:out_of_stock))
 
-  - elsif current_order(false) && !current_order(false).can_add_product_to_cart?(@product)
+  - elsif current_order(false) && !order.can_add_product_to_cart?(@product)
     .error-distributor
       Please complete your order at
       = link_to current_distributor.name, root_path
@@ -14,8 +15,7 @@
     - if @product.group_buy
       %p Max quantity
       = number_field_tag (@product.has_variants? ? :max_quantity : "variant_attributes[#{@product.master.id}][max_quantity]"), 1, :class => 'title max_quantity', :in => 1..@product.on_hand
-    - order = current_order(false)
-    - if order.nil? || order.can_change_distributor?
+    - if order.nil? || DistributorChangeValidator.new(order).can_change_distributor?
       %p Distributor
       = select_tag "distributor_id", options_from_collection_for_select([Enterprise.new]+@product.distributors, "id", "name", current_distributor.andand.id)
     - else

app/views/spree/products/_source_sidebar.html.haml

@@ -9,13 +9,14 @@
   %h6.filter_name Shop by Distributor
   %ul.filter_choices
     - order = current_order(false)
+    - validator = DistributorChangeValidator.new(order)
     - @distributors.each do |distributor|
       %li.nowrap
-        - if order.nil? || order.can_change_to_distributor?(distributor)
+        - if order.nil? || validator.can_change_to_distributor?(distributor)
           = link_to distributor.name, select_distributor_order_path(distributor)
         - elsif order.distributor == distributor
           = link_to distributor.name, [main_app, distributor]
         - else
           %span.inactive= distributor.name
-  - if current_distributor && order.can_change_distributor?
+  - if current_distributor && validator.can_change_distributor?
     = button_to 'Browse All Distributors', deselect_distributor_orders_path, :method => :get

lib/open_food_web/distributor_change_validator.rb

@@ -0,0 +1,22 @@
+class DistributorChangeValidator
+  
+  def initialize order
+    @order = order
+  end
+  
+  def can_change_distributor?
+    # Distributor may not be changed once an item has been added to the cart/order
+    @order.line_items.empty?
+  end
+
+  def can_change_to_distributor? distributor
+    # Distributor may not be changed once an item has been added to the cart/order, unless all items are available from the specified distributor
+    @order.line_items.empty? || (available_distributors(Enterprise.all) || []).include?(distributor)
+  end
+  
+  def available_distributors enterprises
+    enterprises.select do |e|
+      (@order.line_item_variants - e.available_variants).empty?
+    end
+  end
+end

spec/lib/open_food_web/distributor_change_validator_spec.rb

@@ -0,0 +1,80 @@
+require 'open_food_web/distributor_change_validator'
+
+describe DistributorChangeValidator do
+  let(:order) { double(:order) }
+  let(:subject) { DistributorChangeValidator.new(order) }
+  
+  context "permissions for changing distributor" do
+    it "allows distributor to be changed if line_items is empty" do
+      order.stub(:line_items) { [] }
+      subject.can_change_distributor?.should be_true
+    end
+    
+    it "does not allow distributor to be changed if line_items is not empty" do
+      order.stub(:line_items) { [1, 2, 3] }
+      subject.can_change_distributor?.should be_false
+    end
+  end
+  
+  context "finding distributors which have the same variants" do
+    it "matches enterprises which offer all products within the order" do
+      variant1 = double(:variant)
+      variant2 = double(:variant)
+      variant3 = double(:variant)
+      variant5 = double(:variant)
+      line_item_variants = [variant1, variant3, variant5]
+      order.stub(:line_item_variants){ line_item_variants }
+      enterprise = double(:enterprise)
+      enterprise.stub(:available_variants){ line_item_variants } # Exactly the same variants as the order
+
+      subject.available_distributors([enterprise]).should == [enterprise]
+    end
+
+    it "does not match enterprises with no products available" do
+      variant1 = double(:variant)
+      variant3 = double(:variant)
+      variant5 = double(:variant)
+      line_item_variants = [variant1, variant3, variant5]
+      order.stub(:line_item_variants){ line_item_variants }
+      enterprise = double(:enterprise)
+      enterprise.stub(:available_variants){ [] } # No variants
+
+      subject.available_distributors([enterprise]).should_not include enterprise
+    end
+
+    it "does not match enterprises with only some of the same variants in the order available" do
+      variant1 = double(:variant)
+      variant2 = double(:variant)
+      variant3 = double(:variant)
+      variant4 = double(:variant)
+      variant5 = double(:variant)
+      line_item_variants = [variant1, variant3, variant5]
+      order.stub(:line_item_variants){ line_item_variants }
+      enterprise_with_some_variants = double(:enterprise)
+      enterprise_with_some_variants.stub(:available_variants){ [variant1, variant3] } # Only some variants
+      enterprise_with_some_plus_extras = double(:enterprise)
+      enterprise_with_some_plus_extras.stub(:available_variants){ [variant1, variant2, variant3, variant4] } # Only some variants, plus extras
+      
+      subject.available_distributors([enterprise_with_some_variants]).should_not include enterprise_with_some_variants
+      subject.available_distributors([enterprise_with_some_plus_extras]).should_not include enterprise_with_some_plus_extras
+    end
+
+    it "matches enteprises which offer all products in the order, plus additional products" do
+      variant1 = double(:variant)
+      variant2 = double(:variant)
+      variant3 = double(:variant)
+      variant4 = double(:variant)
+      variant5 = double(:variant)
+      line_item_variants = [variant1, variant3, variant5]
+      order.stub(:line_item_variants){ line_item_variants }
+      enterprise = double(:enterprise)
+      enterprise.stub(:available_variants){ [variant1, variant2, variant3, variant4, variant5] } # Excess variants
+      
+      subject.available_distributors([enterprise]).should == [enterprise]
+    end
+    
+    it "matches no enterprises when none are provided" do
+      subject.available_distributors([]).should == []
+    end
+  end
+end

spec/models/order_spec.rb

@@ -14,59 +14,6 @@ describe Spree::Order do
     subject.adjustments.where(:label => "Shipping").should be_present
   end
 
-  it "reveals permission for changing distributor" do
-    d = create(:distributor_enterprise)
-    p = create(:product, :distributors => [d])
-
-    subject.distributor = d
-    subject.save!
-
-    subject.can_change_distributor?.should be_true
-    subject.add_variant(p.master, 1)
-    subject.can_change_distributor?.should be_false
-  end
-
-  it "checks that distributor is available when changing, and raises an exception if distributor is changed without permission" do
-    d = create(:distributor_enterprise)
-    p = create(:product, :distributors => [d])
-    subject.distributor = d
-    subject.save!
-
-    subject.add_variant(p.master, 1)
-    subject.can_change_distributor?.should be_false
-    subject.should_receive(:available_distributors)
-
-    expect do
-      subject.distributor = nil
-    end.to raise_error "You cannot change the distributor of an order with products"
-  end
-
-  it "reveals permission for adding products to the cart" do
-    d1 = create(:distributor_enterprise)
-    d2 = create(:distributor_enterprise)
-
-    p_first = create(:product, :distributors => [d1])
-    p_subsequent_same_dist = create(:product, :distributors => [d1])
-    p_subsequent_other_dist = create(:product, :distributors => [d2])
-
-    # We need to set distributor, since order.add_variant does not, and
-    # we also need to save the order so that line items can be added to
-    # the association.
-    subject.distributor = d1
-    subject.save!
-
-    # The first product in the cart can be added
-    subject.can_add_product_to_cart?(p_first).should be_true
-    subject.add_variant(p_first.master, 1)
-
-    # A subsequent product can be added if the distributor matches
-    subject.can_add_product_to_cart?(p_subsequent_same_dist).should be_true
-    subject.add_variant(p_subsequent_same_dist.master, 1)
-
-    # And cannot be added if it does not match
-    subject.can_add_product_to_cart?(p_subsequent_other_dist).should be_false
-  end
-
   it "sets attributes on line items for variants" do
     d = create(:distributor_enterprise)
     p = create(:product, :distributors => [d])
@@ -81,62 +28,69 @@ describe Spree::Order do
     li.max_quantity.should == 3
   end
 
-  context "finding alternative distributors" do
-    it "checks that variants are available" do
-      distributors_with_all_variants = double(:distributors_with_all_variants)
-      subject.should_receive(:get_distributors_with_all_variants).with(Enterprise.all)
-      subject.available_distributors
+  context "permissions for adding products to the cart" do
+    it "allows products to be added to cart when cart is empty" do
+      p_first = double(:product)
+      p_first.stub(:distributors) { [d1] }
+      subject.stub(:line_items) { [] }
+      subject.can_add_product_to_cart?(p_first).should be_true
     end
 
-    context "finding distributors which have the same variants" do
-      before(:each) do
-        @enterprise1 = FactoryGirl.create(:enterprise, id: 1)
-        subject.distributor = @enterprise1
-        @product1 = FactoryGirl.create(:product)
-        @product2 = FactoryGirl.create(:product)
-        @product3 = FactoryGirl.create(:product)
-        variant11 = FactoryGirl.create(:variant, product: @product1)
-        variant12 = FactoryGirl.create(:variant, product: @product1)
-        variant21 = FactoryGirl.create(:variant, product: @product2)
-        variant31 = FactoryGirl.create(:variant, product: @product3)
-        variant32 = FactoryGirl.create(:variant, product: @product3)
+    it "allows products to be added to cart when they are available from the current distributor" do
+      d1 = double(:distributor)
+      p_first = double(:product)
+      p_first.stub(:distributors) { [d1] }
+      p_subsequent_same_dist = double(:product)
+      p_subsequent_same_dist.stub(:distributors) { [d1] }
+      subject.stub(:line_items) { [1, 2, 3] }
+      subject.stub(:distributor) { d1 }
+      subject.can_add_product_to_cart?(p_subsequent_same_dist).should be_true
+    end
 
-        # Product Distributions
-        # Enterprise 1 sells product 1 and product 3
-        FactoryGirl.create(:product_distribution, product: @product1, distributor: @enterprise1)
-        FactoryGirl.create(:product_distribution, product: @product3, distributor: @enterprise1)
+    it "does not allow products to be added to cart when they are not available from the current distributor" do
+      d1 = double(:distributor)
+      d2 = double(:distributor)
+      p_first = double(:product)
+      p_first.stub(:distributors) { [d1] }
+      p_subsequent_other_dist = double(:product)
+      p_subsequent_other_dist.stub(:distributors) { [d2] }
+      subject.stub(:line_items) { [1, 2, 3] }
+      subject.stub(:distributor) { d1 }
+      subject.can_add_product_to_cart?(p_subsequent_other_dist).should be_false
+    end
+  end
 
-        # Build the current order
-        line_item1 = FactoryGirl.create(:line_item, order: subject, variant: variant11)
-        line_item2 = FactoryGirl.create(:line_item, order: subject, variant: variant12)
-        line_item3 = FactoryGirl.create(:line_item, order: subject, variant: variant31)
-        subject.line_items = [line_item1,line_item2,line_item3]
-      end
+  context "validating distributor changes" do
+    it "checks that a distributor is available when changing" do
+      order_enterprise = FactoryGirl.create(:enterprise, id: 1, :name => "Order Enterprise")
+      subject.distributor = order_enterprise
+      product1 = FactoryGirl.create(:product)
+      product2 = FactoryGirl.create(:product)
+      product3 = FactoryGirl.create(:product)
+      variant11 = FactoryGirl.create(:variant, product: product1)
+      variant12 = FactoryGirl.create(:variant, product: product1)
+      variant21 = FactoryGirl.create(:variant, product: product2)
+      variant31 = FactoryGirl.create(:variant, product: product3)
+      variant32 = FactoryGirl.create(:variant, product: product3)
 
-      it "matches the distributor enterprise of the current order" do
-        subject.get_distributors_with_all_variants([@enterprise1]).should == [@enterprise1]
-      end
+      # Product Distributions
+      # Order Enterprise sells product 1 and product 3
+      FactoryGirl.create(:product_distribution, product: product1, distributor: order_enterprise)
+      FactoryGirl.create(:product_distribution, product: product3, distributor: order_enterprise)
 
-      it "does not match enterprises with no products available" do
-        test_enterprise = FactoryGirl.create(:enterprise, id: 2)
-        subject.get_distributors_with_all_variants([@enterprise1, test_enterprise]).should_not include test_enterprise
-      end
+      # Build the current order
+      line_item1 = FactoryGirl.create(:line_item, order: subject, variant: variant11)
+      line_item2 = FactoryGirl.create(:line_item, order: subject, variant: variant12)
+      line_item3 = FactoryGirl.create(:line_item, order: subject, variant: variant31)
+      subject.line_items = [line_item1,line_item2,line_item3]
 
-      it "does not match enterprises with only some of the same variants in the current order available" do
-        test_enterprise = FactoryGirl.create(:enterprise, id: 2)
-        # Test Enterprise sells only product 1
-        FactoryGirl.create(:product_distribution, product: @product1, distributor: test_enterprise)
-        subject.get_distributors_with_all_variants([@enterprise1, test_enterprise]).should_not include test_enterprise
-      end
+      test_enterprise = FactoryGirl.create(:enterprise, id: 2, :name => "Test Enterprise")
+      # Test Enterprise sells only product 1
+      FactoryGirl.create(:product_distribution, product: product1, distributor: test_enterprise)
 
-      it "matches enteprises which offer all products in the current order" do
-        test_enterprise = FactoryGirl.create(:enterprise, id: 2)
-        # Enterprise 3 Sells Products 1, 2 and 3
-        FactoryGirl.create(:product_distribution, product: @product1, distributor: test_enterprise)
-        FactoryGirl.create(:product_distribution, product: @product2, distributor: test_enterprise)
-        FactoryGirl.create(:product_distribution, product: @product3, distributor: test_enterprise)
-        subject.get_distributors_with_all_variants([@enterprise1, test_enterprise]).should include test_enterprise
-     end
+      subject.distributor = test_enterprise
+      subject.should_not be_valid
+      subject.errors.should include :distributor_id
     end
   end
 end