From 8c9a3c8a912818654c2dc37bbe8845bcbe28cf31 Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Tue, 6 Aug 2013 14:56:33 +1000
Subject: [PATCH 1/8] Add Product#product_distribution_for

---
 app/models/spree/product_decorator.rb | 6 +++++-
 spec/models/product_spec.rb           | 9 ++++++++-
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb
index f0405baed6..e11b0c2375 100644
--- a/app/models/spree/product_decorator.rb
+++ b/app/models/spree/product_decorator.rb
@@ -76,6 +76,10 @@ Spree::Product.class_eval do
     self.class.in_order_cycle(order_cycle).include? self
   end
 
+  def product_distribution_for(distributor)
+    self.product_distributions.find_by_distributor_id(distributor)
+  end
+
   # This method is called on products that are distributed via order cycles, but at the time of
   # writing (27-5-2013), order cycle fees were not implemented, so there's no defined result
   # that this method should return. As a stopgap, we notify Bugsnag of the situation and return
@@ -83,7 +87,7 @@ Spree::Product.class_eval do
   # should return the order cycle shipping method, or raise an exepction with the message,
   # "This product is not available through that distributor".
   def shipping_method_for_distributor(distributor)
-    distribution = self.product_distributions.find_by_distributor_id(distributor)
+    distribution = product_distribution_for distributor
 
     unless distribution
       Bugsnag.notify(Exception.new "No product distribution for product #{id} at distributor #{distributor.andand.id}. Perhaps this product is distributed via an order cycle? This is a warning that OrderCycle fees and shipping methods are not yet implemented, and the shipping fee charged is undefined until then.")
diff --git a/spec/models/product_spec.rb b/spec/models/product_spec.rb
index 8704dce6be..b6d9e5ac36 100644
--- a/spec/models/product_spec.rb
+++ b/spec/models/product_spec.rb
@@ -155,7 +155,7 @@ module Spree
         end
       end
 
-      describe 'access roles' do
+      describe "access roles" do
         before(:each) do
           @e1 = create(:enterprise)
           @e2 = create(:enterprise)
@@ -185,6 +185,13 @@ module Spree
     end
 
     describe "finders" do
+      it "finds the product distribution for a particular distributor" do
+        distributor = create(:distributor_enterprise)
+        product = create(:product)
+        product_distribution = create(:product_distribution, product: product, distributor: distributor)
+        product.product_distribution_for(distributor).should == product_distribution
+      end
+
       it "finds the shipping method for a particular distributor" do
         shipping_method = create(:shipping_method)
         distributor = create(:distributor_enterprise)

From a338c974f816cdb260b435b0fb79e601e0cb7637 Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Thu, 8 Aug 2013 10:38:59 +1000
Subject: [PATCH 2/8] Create line item adjustments for product distributions

---
 app/models/product_distribution.rb       |  26 +++++
 app/models/spree/order_decorator.rb      |  15 +++
 spec/factories.rb                        |   1 +
 spec/models/order_spec.rb                |  31 ++++++
 spec/models/product_distribution_spec.rb | 131 +++++++++++++++++++++++
 5 files changed, 204 insertions(+)

diff --git a/app/models/product_distribution.rb b/app/models/product_distribution.rb
index eefde4dd63..713f640fad 100644
--- a/app/models/product_distribution.rb
+++ b/app/models/product_distribution.rb
@@ -7,4 +7,30 @@ class ProductDistribution < ActiveRecord::Base
   validates_presence_of :product_id, :on => :update
   validates_presence_of :distributor_id, :shipping_method_id
   validates_uniqueness_of :product_id, :scope => :distributor_id
+
+
+  def ensure_correct_adjustment_for(line_item)
+    if enterprise_fee
+      adjustment = adjustment_on line_item
+      create_adjustment_on line_item unless adjustment
+    end
+  end
+
+  def adjustment_on(line_item)
+    adjustments = line_item.adjustments.where(source_id: enterprise_fee)
+
+    raise "Multiple adjustments for this enterprise fee on this line item. This method is not designed to deal with this scenario." if adjustments.count > 1
+
+    adjustments.first
+  end
+
+  def create_adjustment_on(line_item)
+    enterprise_fee.create_adjustment(adjustment_label, line_item, enterprise_fee, true)
+  end
+
+
+  def adjustment_label
+    "Product distribution by #{distributor.name}"
+  end
+
 end
diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb
index a5f5f87c0d..f5d05b429c 100644
--- a/app/models/spree/order_decorator.rb
+++ b/app/models/spree/order_decorator.rb
@@ -13,6 +13,8 @@ Spree::Order.class_eval do
   before_save :update_line_item_shipping_methods
   after_create :set_default_shipping_method
 
+  register_update_hook :update_distribution_charge!
+
   # -- Scopes
   scope :managed_by, lambda { |user|
     if user.has_spree_role?('admin')
@@ -22,6 +24,7 @@ Spree::Order.class_eval do
     end
   }
 
+
   # -- Methods
   def products_available_from_new_distribution
     # Check that the line_items in the current order are available from a newly selected distribution
@@ -56,6 +59,13 @@ Spree::Order.class_eval do
     save!
   end
 
+  def update_distribution_charge!
+    line_items.each do |line_item|
+      pd = product_distribution_for line_item
+      pd.ensure_correct_adjustment_for line_item
+    end
+  end
+
   def set_variant_attributes(variant, attributes)
     line_item = find_line_item_by_variant(variant)
 
@@ -113,4 +123,9 @@ Spree::Order.class_eval do
       self.update!
     end
   end
+
+  def product_distribution_for(line_item)
+    line_item.variant.product.product_distribution_for self.distributor
+  end
+
 end
diff --git a/spec/factories.rb b/spec/factories.rb
index 4d91282be1..32a9cd770a 100644
--- a/spec/factories.rb
+++ b/spec/factories.rb
@@ -89,6 +89,7 @@ FactoryGirl.define do
     product         { |pd| Spree::Product.first || FactoryGirl.create(:product) }
     distributor     { |pd| Enterprise.is_distributor.first || FactoryGirl.create(:distributor_enterprise) }
     shipping_method { |pd| Spree::ShippingMethod.where("name != 'Delivery'").first || FactoryGirl.create(:shipping_method) }
+    enterprise_fee  { |pd| FactoryGirl.create(:enterprise_fee, enterprise: pd.distributor) }
   end
 
   factory :itemwise_shipping_method, :parent => :shipping_method do
diff --git a/spec/models/order_spec.rb b/spec/models/order_spec.rb
index f9dca32470..016b23099b 100644
--- a/spec/models/order_spec.rb
+++ b/spec/models/order_spec.rb
@@ -28,6 +28,37 @@ describe Spree::Order do
     li.max_quantity.should == 3
   end
 
+  describe "updating the distribution charge" do
+    let(:order) { build(:order) }
+
+    it "updates distribution charge after save" do
+      order.should_receive(:update_distribution_charge!).at_least(:once)
+      order.save!
+    end
+
+    it "ensures the correct adjustment(s) are created for the product distribution" do
+      line_item = double(:line_item)
+      subject.stub(:line_items) { [line_item] }
+
+      product_distribution = double(:product_distribution)
+      product_distribution.should_receive(:ensure_correct_adjustment_for).with(line_item)
+      subject.stub(:product_distribution_for) { product_distribution }
+
+      subject.send(:update_distribution_charge!)
+    end
+
+    it "looks up product distribution enterprise fees for a line item" do
+      product = double(:product)
+      variant = double(:variant, product: product)
+      line_item = double(:line_item, variant: variant)
+
+      product_distribution = double(:product_distribution)
+      product.should_receive(:product_distribution_for).with(subject.distributor) { product_distribution }
+
+      subject.send(:product_distribution_for, line_item).should == product_distribution
+    end
+  end
+
   describe "setting the distributor" do
     it "sets the distributor when no order cycle is set" do
       d = create(:distributor_enterprise)
diff --git a/spec/models/product_distribution_spec.rb b/spec/models/product_distribution_spec.rb
index 51b5b9a8a2..0eba225f38 100644
--- a/spec/models/product_distribution_spec.rb
+++ b/spec/models/product_distribution_spec.rb
@@ -20,4 +20,135 @@ describe ProductDistribution do
     pd2 = build(:product_distribution, :product => new_product, :distributor => new_distributor)
     pd2.should be_valid
   end
+
+
+  describe "adjusting orders" do
+    context "integration" do
+      it "creates an adjustment for product distributions" do
+        # Given an order
+        distributor = create(:distributor_enterprise)
+        order = create(:order, distributor: distributor)
+
+        # And a product with a product distribution with an enterprise fee
+        product = create(:product)
+        enterprise_fee = create(:enterprise_fee, calculator: build(:calculator))
+        enterprise_fee.calculator.preferred_amount = 1.23
+        enterprise_fee.calculator.save!
+        create(:product_distribution, product: product, distributor: distributor, enterprise_fee: enterprise_fee)
+
+        # When I add the product to the order, an adjustment should be made
+        expect do
+          op = Spree::OrderPopulator.new order, 'AU'
+          op.populate products: {product.id => product.master.id}, quantity: 1, distributor_id: distributor.id
+        end.to change(Spree::Adjustment, :count).by(1)
+
+        # And it should have the correct data
+        order.reload
+        order.line_items.count.should == 1
+        order.line_items.last.adjustments.count.should == 1
+        adjustment = order.line_items.last.adjustments.last
+
+        adjustment.source.should == enterprise_fee
+        adjustment.originator.should == enterprise_fee
+        adjustment.label.should == "Product distribution by #{distributor.name}"
+        adjustment.amount.should == 1.23
+
+        # And it should have some associated metadata
+        pending 'Needs metadata spec'
+      end
+    end
+
+    describe "ensuring that a line item has the correct adjustment" do
+      let(:enterprise_fee) { EnterpriseFee.new }
+      let(:pd) { ProductDistribution.new enterprise_fee: enterprise_fee }
+      let(:line_item) { double(:line_item) }
+      let(:adjustment) { double(:adjustment) }
+
+      # TODO: This spec will go away once enterprise_fee is required
+      it "does nothing if there is no enterprise fee set" do
+        pd.enterprise_fee = nil
+        pd.should_receive(:adjustment_on).never
+        pd.ensure_correct_adjustment_for line_item
+      end
+
+      describe "adding items to cart" do
+        it "creates an adjustment for the new item" do
+          pd.stub(:adjustment_on) { nil }
+          pd.should_receive(:create_adjustment_on).with(line_item)
+
+          pd.ensure_correct_adjustment_for line_item
+        end
+
+        it "makes no change to the adjustment of existing items" do
+          pd.stub(:adjustment_on) { adjustment }
+          pd.should_receive(:create_adjustment_on).never
+
+          pd.ensure_correct_adjustment_for line_item
+        end
+      end
+
+      describe "changing distributor" do
+        it "clears and re-creates the adjustment on the line item"
+      end
+    end
+
+    describe "finding our adjustment on a line item" do
+      it "returns nil when not present" do
+        line_item = build(:line_item)
+        pd = ProductDistribution.new
+        pd.send(:adjustment_on, line_item).should be_nil
+      end
+
+      it "returns the adjustment when present" do
+        # TODO: This spec can be simplified (ie. use the default ProductDistribution factory)
+        # once Spree's calculators are compatible with LineItem targets, not just Orders.
+        distributor = create(:distributor_enterprise)
+        enterprise_fee = create(:enterprise_fee, enterprise: distributor, calculator: build(:calculator))
+
+        pd = create(:product_distribution, distributor: distributor, enterprise_fee: enterprise_fee)
+        line_item = create(:line_item)
+        adjustment = pd.enterprise_fee.create_adjustment('foo', line_item, pd.enterprise_fee, true)
+
+        pd.send(:adjustment_on, line_item).should == adjustment
+      end
+
+      it "raises an error when there are multiple adjustments for this enterprise fee" do
+        # TODO: This spec can be simplified (ie. use the default ProductDistribution factory)
+        # once Spree's calculators are compatible with LineItem targets, not just Orders.
+        distributor = create(:distributor_enterprise)
+        enterprise_fee = create(:enterprise_fee, enterprise: distributor, calculator: build(:calculator))
+
+        pd = create(:product_distribution, distributor: distributor, enterprise_fee: enterprise_fee)
+        line_item = create(:line_item)
+        pd.enterprise_fee.create_adjustment('one', line_item, pd.enterprise_fee, true)
+        pd.enterprise_fee.create_adjustment('two', line_item, pd.enterprise_fee, true)
+
+        expect do
+          pd.send(:adjustment_on, line_item)
+        end.to raise_error "Multiple adjustments for this enterprise fee on this line item. This method is not designed to deal with this scenario."
+      end
+    end
+
+    describe "creating an adjustment on a line item" do
+      it "creates the adjustment via the enterprise fee" do
+        # TODO: This spec can be simplified (ie. use the default ProductDistribution factory)
+        # once Spree's calculators are compatible with LineItem targets, not just Orders.
+        distributor = create(:distributor_enterprise)
+        enterprise_fee = create(:enterprise_fee, enterprise: distributor, calculator: build(:calculator))
+        pd = create(:product_distribution, distributor: distributor, enterprise_fee: enterprise_fee)
+
+        pd.stub(:adjustment_label) { 'label' }
+        line_item = create(:line_item)
+
+        expect { pd.send(:create_adjustment_on, line_item) }.to change(Spree::Adjustment, :count).by(1)
+
+        adjustment = Spree::Adjustment.last
+        adjustment.label.should == 'label'
+        adjustment.adjustable.should == line_item
+        adjustment.source.should == enterprise_fee
+        adjustment.originator.should == enterprise_fee
+        adjustment.should be_mandatory
+      end
+    end
+  end
 end

From 239dd29511ee958036adfbc1e22d0b1b3e881e8f Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Thu, 8 Aug 2013 11:18:42 +1000
Subject: [PATCH 3/8] Weight calculator will calculate against a single line
 item as well as an order

---
 app/models/open_food_web/calculator/weight.rb | 16 +++++++++++++++-
 spec/models/calculator/weight_spec.rb         |  9 +++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/app/models/open_food_web/calculator/weight.rb b/app/models/open_food_web/calculator/weight.rb
index f927988886..049f4ad15f 100644
--- a/app/models/open_food_web/calculator/weight.rb
+++ b/app/models/open_food_web/calculator/weight.rb
@@ -8,8 +8,22 @@ module OpenFoodWeb
     end
 
     def compute(object)
-      total_weight = object.line_items.inject(0) { |sum, li| sum + ((li.variant.andand.weight || 0) * li.quantity) }
+      line_items = line_items_for object
+      total_weight = line_items.sum { |li| ((li.variant.andand.weight || 0) * li.quantity) }
       total_weight * self.preferred_per_kg
     end
+
+
+    private
+
+    def line_items_for(object)
+      if object.respond_to? :line_items
+        object.line_items
+      elsif object.respond_to?(:variant) && object.respond_to?(:quantity)
+        [object]
+      else
+        raise "Unknown object type: #{object.inspect}"
+      end
+    end
   end
 end
diff --git a/spec/models/calculator/weight_spec.rb b/spec/models/calculator/weight_spec.rb
index acd8480d12..3b71a0787b 100644
--- a/spec/models/calculator/weight_spec.rb
+++ b/spec/models/calculator/weight_spec.rb
@@ -15,4 +15,13 @@ describe OpenFoodWeb::Calculator::Weight do
     subject.set_preference(:per_kg, 10)
     subject.compute(order).should == (10*1 + 20*3) * 10
   end
+
+  it "computes shipping cost for a line item" do
+    variant = double(:variant, :weight => 10)
+
+    line_item = double(:line_item, :variant => variant, :quantity => 2)
+
+    subject.set_preference(:per_kg, 10)
+    subject.compute(line_item).should == 10*2 * 10
+  end
 end

From fd989e3a77770fc157bf17661134983094fde8ad Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Thu, 8 Aug 2013 11:20:30 +1000
Subject: [PATCH 4/8] Create adjustments so computation is performed against
 line item. Simplify specs now that weight calculator is compatible with line
 items.

---
 app/models/product_distribution.rb       |  5 ++--
 spec/models/product_distribution_spec.rb | 33 +++++++-----------------
 2 files changed, 11 insertions(+), 27 deletions(-)

diff --git a/app/models/product_distribution.rb b/app/models/product_distribution.rb
index 713f640fad..490ea0f1a2 100644
--- a/app/models/product_distribution.rb
+++ b/app/models/product_distribution.rb
@@ -17,7 +17,7 @@ class ProductDistribution < ActiveRecord::Base
   end
 
   def adjustment_on(line_item)
-    adjustments = line_item.adjustments.where(source_id: enterprise_fee)
+    adjustments = line_item.adjustments.where(originator_id: enterprise_fee)
 
     raise "Multiple adjustments for this enterprise fee on this line item. This method is not designed to deal with this scenario." if adjustments.count > 1
 
@@ -25,10 +25,9 @@ class ProductDistribution < ActiveRecord::Base
   end
 
   def create_adjustment_on(line_item)
-    enterprise_fee.create_adjustment(adjustment_label, line_item, enterprise_fee, true)
+    enterprise_fee.create_adjustment(adjustment_label, line_item, line_item, true)
   end
 
-
   def adjustment_label
     "Product distribution by #{distributor.name}"
   end
diff --git a/spec/models/product_distribution_spec.rb b/spec/models/product_distribution_spec.rb
index 0eba225f38..22c8621c57 100644
--- a/spec/models/product_distribution_spec.rb
+++ b/spec/models/product_distribution_spec.rb
@@ -48,7 +48,7 @@ describe ProductDistribution do
         order.line_items.last.adjustments.count.should == 1
         adjustment = order.line_items.last.adjustments.last
 
-        adjustment.source.should == enterprise_fee
+        adjustment.source.should == order.line_items.last
         adjustment.originator.should == enterprise_fee
         adjustment.label.should == "Product distribution by #{distributor.name}"
         adjustment.amount.should == 1.23
@@ -100,28 +100,18 @@ describe ProductDistribution do
       end
 
       it "returns the adjustment when present" do
-        # TODO: This spec can be simplified (ie. use the default ProductDistribution factory)
-        # once Spree's calculators are compatible with LineItem targets, not just Orders.
-        distributor = create(:distributor_enterprise)
-        enterprise_fee = create(:enterprise_fee, enterprise: distributor, calculator: build(:calculator))
-
-        pd = create(:product_distribution, distributor: distributor, enterprise_fee: enterprise_fee)
+        pd = create(:product_distribution)
         line_item = create(:line_item)
-        adjustment = pd.enterprise_fee.create_adjustment('foo', line_item, pd.enterprise_fee, true)
+        adjustment = pd.enterprise_fee.create_adjustment('foo', line_item, line_item, true)
 
         pd.send(:adjustment_on, line_item).should == adjustment
       end
 
       it "raises an error when there are multiple adjustments for this enterprise fee" do
-        # TODO: This spec can be simplified (ie. use the default ProductDistribution factory)
-        # once Spree's calculators are compatible with LineItem targets, not just Orders.
-        distributor = create(:distributor_enterprise)
-        enterprise_fee = create(:enterprise_fee, enterprise: distributor, calculator: build(:calculator))
-
-        pd = create(:product_distribution, distributor: distributor, enterprise_fee: enterprise_fee)
+        pd = create(:product_distribution)
         line_item = create(:line_item)
-        pd.enterprise_fee.create_adjustment('one', line_item, pd.enterprise_fee, true)
-        pd.enterprise_fee.create_adjustment('two', line_item, pd.enterprise_fee, true)
+        pd.enterprise_fee.create_adjustment('one', line_item, line_item, true)
+        pd.enterprise_fee.create_adjustment('two', line_item, line_item, true)
 
         expect do
           pd.send(:adjustment_on, line_item)
@@ -131,12 +121,7 @@ describe ProductDistribution do
 
     describe "creating an adjustment on a line item" do
       it "creates the adjustment via the enterprise fee" do
-        # TODO: This spec can be simplified (ie. use the default ProductDistribution factory)
-        # once Spree's calculators are compatible with LineItem targets, not just Orders.
-        distributor = create(:distributor_enterprise)
-        enterprise_fee = create(:enterprise_fee, enterprise: distributor, calculator: build(:calculator))
-        pd = create(:product_distribution, distributor: distributor, enterprise_fee: enterprise_fee)
-
+        pd = create(:product_distribution)
         pd.stub(:adjustment_label) { 'label' }
         line_item = create(:line_item)
 
@@ -145,8 +130,8 @@ describe ProductDistribution do
         adjustment = Spree::Adjustment.last
         adjustment.label.should == 'label'
         adjustment.adjustable.should == line_item
-        adjustment.source.should == enterprise_fee
-        adjustment.originator.should == enterprise_fee
+        adjustment.source.should == line_item
+        adjustment.originator.should == pd.enterprise_fee
         adjustment.should be_mandatory
       end
     end

From 513993324dea37884b333e4578362803b3c682ac Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Thu, 8 Aug 2013 13:37:40 +1000
Subject: [PATCH 5/8] Clean up old adjustments when changing distributor

---
 app/models/product_distribution.rb       |  8 +++-
 spec/models/product_distribution_spec.rb | 60 ++++++++++++++++++++----
 2 files changed, 57 insertions(+), 11 deletions(-)

diff --git a/app/models/product_distribution.rb b/app/models/product_distribution.rb
index 490ea0f1a2..42354ff636 100644
--- a/app/models/product_distribution.rb
+++ b/app/models/product_distribution.rb
@@ -11,8 +11,8 @@ class ProductDistribution < ActiveRecord::Base
 
   def ensure_correct_adjustment_for(line_item)
     if enterprise_fee
-      adjustment = adjustment_on line_item
-      create_adjustment_on line_item unless adjustment
+      clear_all_enterprise_fee_adjustments_on line_item
+      create_adjustment_on line_item
     end
   end
 
@@ -28,6 +28,10 @@ class ProductDistribution < ActiveRecord::Base
     enterprise_fee.create_adjustment(adjustment_label, line_item, line_item, true)
   end
 
+  def clear_all_enterprise_fee_adjustments_on(line_item)
+    line_item.adjustments.where(originator_type: 'EnterpriseFee').destroy_all
+  end
+
   def adjustment_label
     "Product distribution by #{distributor.name}"
   end
diff --git a/spec/models/product_distribution_spec.rb b/spec/models/product_distribution_spec.rb
index 22c8621c57..e649b3918a 100644
--- a/spec/models/product_distribution_spec.rb
+++ b/spec/models/product_distribution_spec.rb
@@ -72,23 +72,37 @@ describe ProductDistribution do
       end
 
       describe "adding items to cart" do
-        it "creates an adjustment for the new item" do
-          pd.stub(:adjustment_on) { nil }
-          pd.should_receive(:create_adjustment_on).with(line_item)
-
+        it "clears all enterprise fee adjustments on the line item" do
+          pd.should_receive(:clear_all_enterprise_fee_adjustments_on).with(line_item)
+          pd.stub(:create_adjustment_on)
           pd.ensure_correct_adjustment_for line_item
         end
 
-        it "makes no change to the adjustment of existing items" do
-          pd.stub(:adjustment_on) { adjustment }
-          pd.should_receive(:create_adjustment_on).never
-
+        it "creates an adjustment on the line item" do
+          pd.stub(:clear_all_enterprise_fee_adjustments_on)
+          pd.should_receive(:create_adjustment_on).with(line_item)
           pd.ensure_correct_adjustment_for line_item
         end
       end
 
       describe "changing distributor" do
-        it "clears and re-creates the adjustment on the line item"
+        it "clears and re-creates the adjustment on the line item" do
+          # Given a line item with an adjustment via one enterprise fee
+          p = create(:simple_product)
+          d1, d2 = create(:distributor_enterprise), create(:distributor_enterprise)
+          pd1 = create(:product_distribution, product: p, distributor: d1)
+          pd2 = create(:product_distribution, product: p, distributor: d2)
+          line_item = create(:line_item, product: p)
+          pd1.enterprise_fee.create_adjustment('foo', line_item, line_item, true)
+
+          # When I ensure correct adjustment through the other product distribution
+          pd2.ensure_correct_adjustment_for line_item
+
+          # Then I should have only an adjustment originating from the other product distribution
+          line_item.reload
+          line_item.adjustments.count.should == 1
+          line_item.adjustments.first.originator.should == pd2.enterprise_fee
+        end
       end
     end
 
@@ -135,5 +149,33 @@ describe ProductDistribution do
         adjustment.should be_mandatory
       end
     end
+
+    describe "clearing all enterprise fee adjustments on a line item" do
+      it "clears adjustments originating from many different enterprise fees" do
+        p = create(:simple_product)
+        d1, d2 = create(:distributor_enterprise), create(:distributor_enterprise)
+        pd1 = create(:product_distribution, product: p, distributor: d1)
+        pd2 = create(:product_distribution, product: p, distributor: d2)
+        line_item = create(:line_item, product: p)
+        pd1.enterprise_fee.create_adjustment('foo1', line_item, line_item, true)
+        pd2.enterprise_fee.create_adjustment('foo2', line_item, line_item, true)
+
+        expect do
+          pd1.send(:clear_all_enterprise_fee_adjustments_on, line_item)
+        end.to change(line_item.adjustments, :count).by(-2)
+      end
+
+      it "does not clear adjustments originating from another source" do
+        p = create(:simple_product)
+        pd = create(:product_distribution)
+        line_item = create(:line_item, product: pd.product)
+        tax_rate = create(:tax_rate, calculator: build(:calculator, preferred_amount: 10))
+        tax_rate.create_adjustment('foo', line_item, line_item)
+
+        expect do
+          pd.send(:clear_all_enterprise_fee_adjustments_on, line_item)
+        end.to change(line_item.adjustments, :count).by(0)
+      end
+    end
   end
 end

From b4a7ccf1b407aa568bbb4746595df9914dee44e9 Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Fri, 9 Aug 2013 14:14:48 +1000
Subject: [PATCH 6/8] Adjustments on LineItems don't modify the order total.
 Make adjustments on Order instead.

---
 app/models/product_distribution.rb       | 20 +++---
 app/models/spree/adjustment_decorator.rb |  5 ++
 app/models/spree/order_decorator.rb      |  6 +-
 spec/models/order_spec.rb                |  5 --
 spec/models/product_distribution_spec.rb | 81 ++++++++++++++----------
 5 files changed, 66 insertions(+), 51 deletions(-)
 create mode 100644 app/models/spree/adjustment_decorator.rb

diff --git a/app/models/product_distribution.rb b/app/models/product_distribution.rb
index 42354ff636..7ef2a90cdf 100644
--- a/app/models/product_distribution.rb
+++ b/app/models/product_distribution.rb
@@ -11,29 +11,29 @@ class ProductDistribution < ActiveRecord::Base
 
   def ensure_correct_adjustment_for(line_item)
     if enterprise_fee
-      clear_all_enterprise_fee_adjustments_on line_item
-      create_adjustment_on line_item
+      clear_all_enterprise_fee_adjustments_for line_item
+      create_adjustment_for line_item
     end
   end
 
-  def adjustment_on(line_item)
-    adjustments = line_item.adjustments.where(originator_id: enterprise_fee)
+  def adjustment_for(line_item)
+    adjustments = line_item.order.adjustments.enterprise_fee.where(originator_id: enterprise_fee)
 
     raise "Multiple adjustments for this enterprise fee on this line item. This method is not designed to deal with this scenario." if adjustments.count > 1
 
     adjustments.first
   end
 
-  def create_adjustment_on(line_item)
-    enterprise_fee.create_adjustment(adjustment_label, line_item, line_item, true)
+  def create_adjustment_for(line_item)
+    enterprise_fee.create_adjustment(adjustment_label_for(line_item), line_item.order, line_item, true)
   end
 
-  def clear_all_enterprise_fee_adjustments_on(line_item)
-    line_item.adjustments.where(originator_type: 'EnterpriseFee').destroy_all
+  def clear_all_enterprise_fee_adjustments_for(line_item)
+    line_item.order.adjustments.where(originator_type: 'EnterpriseFee', source_id: line_item, source_type: 'Spree::LineItem').destroy_all
   end
 
-  def adjustment_label
-    "Product distribution by #{distributor.name}"
+  def adjustment_label_for(line_item)
+    "Product distribution by #{distributor.name} for #{line_item.product.name}"
   end
 
 end
diff --git a/app/models/spree/adjustment_decorator.rb b/app/models/spree/adjustment_decorator.rb
new file mode 100644
index 0000000000..6d089ee3b3
--- /dev/null
+++ b/app/models/spree/adjustment_decorator.rb
@@ -0,0 +1,5 @@
+module Spree
+  Adjustment.class_eval do
+    scope :enterprise_fee, where(originator_type: 'EnterpriseFee')
+  end
+end
diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb
index f5d05b429c..290fb45a83 100644
--- a/app/models/spree/order_decorator.rb
+++ b/app/models/spree/order_decorator.rb
@@ -1,5 +1,9 @@
 require 'open_food_web/distribution_change_validator'
 
+ActiveSupport::Notifications.subscribe('spree.order.contents_changed') do |name, start, finish, id, payload|
+  payload[:order].reload.update_distribution_charge!
+end
+
 Spree::Order.class_eval do
   belongs_to :order_cycle
   belongs_to :distributor, :class_name => 'Enterprise'
@@ -13,8 +17,6 @@ Spree::Order.class_eval do
   before_save :update_line_item_shipping_methods
   after_create :set_default_shipping_method
 
-  register_update_hook :update_distribution_charge!
-
   # -- Scopes
   scope :managed_by, lambda { |user|
     if user.has_spree_role?('admin')
diff --git a/spec/models/order_spec.rb b/spec/models/order_spec.rb
index 016b23099b..eb3a968a5c 100644
--- a/spec/models/order_spec.rb
+++ b/spec/models/order_spec.rb
@@ -31,11 +31,6 @@ describe Spree::Order do
   describe "updating the distribution charge" do
     let(:order) { build(:order) }
 
-    it "updates distribution charge after save" do
-      order.should_receive(:update_distribution_charge!).at_least(:once)
-      order.save!
-    end
-
     it "ensures the correct adjustment(s) are created for the product distribution" do
       line_item = double(:line_item)
       subject.stub(:line_items) { [line_item] }
diff --git a/spec/models/product_distribution_spec.rb b/spec/models/product_distribution_spec.rb
index e649b3918a..a1f92d2cb3 100644
--- a/spec/models/product_distribution_spec.rb
+++ b/spec/models/product_distribution_spec.rb
@@ -30,7 +30,7 @@ describe ProductDistribution do
         order = create(:order, distributor: distributor)
 
         # And a product with a product distribution with an enterprise fee
-        product = create(:product)
+        product = create(:product, name: 'Pear')
         enterprise_fee = create(:enterprise_fee, calculator: build(:calculator))
         enterprise_fee.calculator.preferred_amount = 1.23
         enterprise_fee.calculator.save!
@@ -40,17 +40,20 @@ describe ProductDistribution do
         expect do
           op = Spree::OrderPopulator.new order, 'AU'
           op.populate products: {product.id => product.master.id}, quantity: 1, distributor_id: distributor.id
+
+          # Normally the controller would fire this event when the order's contents are changed
+          fire_order_contents_changed_event(order.user, order)
         end.to change(Spree::Adjustment, :count).by(1)
 
         # And it should have the correct data
         order.reload
-        order.line_items.count.should == 1
-        order.line_items.last.adjustments.count.should == 1
-        adjustment = order.line_items.last.adjustments.last
+        adjustments = order.adjustments.where(:originator_type => 'EnterpriseFee')
+        adjustments.count.should == 1
+        adjustment = adjustments.first
 
         adjustment.source.should == order.line_items.last
         adjustment.originator.should == enterprise_fee
-        adjustment.label.should == "Product distribution by #{distributor.name}"
+        adjustment.label.should == "Product distribution by #{distributor.name} for Pear"
         adjustment.amount.should == 1.23
 
         # And it should have some associated metadata
@@ -67,102 +70,104 @@ describe ProductDistribution do
       # TODO: This spec will go away once enterprise_fee is required
       it "does nothing if there is no enterprise fee set" do
         pd.enterprise_fee = nil
-        pd.should_receive(:adjustment_on).never
+        pd.should_receive(:clear_all_enterprise_fee_adjustments_for).never
+        pd.should_receive(:create_adjustment_for).never
         pd.ensure_correct_adjustment_for line_item
       end
 
       describe "adding items to cart" do
         it "clears all enterprise fee adjustments on the line item" do
-          pd.should_receive(:clear_all_enterprise_fee_adjustments_on).with(line_item)
-          pd.stub(:create_adjustment_on)
+          pd.should_receive(:clear_all_enterprise_fee_adjustments_for).with(line_item)
+          pd.stub(:create_adjustment_for)
           pd.ensure_correct_adjustment_for line_item
         end
 
         it "creates an adjustment on the line item" do
-          pd.stub(:clear_all_enterprise_fee_adjustments_on)
-          pd.should_receive(:create_adjustment_on).with(line_item)
+          pd.stub(:clear_all_enterprise_fee_adjustments_for)
+          pd.should_receive(:create_adjustment_for).with(line_item)
           pd.ensure_correct_adjustment_for line_item
         end
       end
 
       describe "changing distributor" do
-        it "clears and re-creates the adjustment on the line item" do
+        it "clears and re-creates the adjustment for the line item" do
           # Given a line item with an adjustment via one enterprise fee
           p = create(:simple_product)
           d1, d2 = create(:distributor_enterprise), create(:distributor_enterprise)
           pd1 = create(:product_distribution, product: p, distributor: d1)
           pd2 = create(:product_distribution, product: p, distributor: d2)
           line_item = create(:line_item, product: p)
-          pd1.enterprise_fee.create_adjustment('foo', line_item, line_item, true)
+          pd1.enterprise_fee.create_adjustment('foo', line_item.order, line_item, true)
 
           # When I ensure correct adjustment through the other product distribution
           pd2.ensure_correct_adjustment_for line_item
 
           # Then I should have only an adjustment originating from the other product distribution
-          line_item.reload
-          line_item.adjustments.count.should == 1
-          line_item.adjustments.first.originator.should == pd2.enterprise_fee
+          line_item.order.reload
+          adjustments = line_item.order.adjustments.enterprise_fee
+          adjustments.count.should == 1
+          adjustments.first.originator.should == pd2.enterprise_fee
         end
       end
     end
 
-    describe "finding our adjustment on a line item" do
+    describe "finding our adjustment for a line item" do
       it "returns nil when not present" do
         line_item = build(:line_item)
         pd = ProductDistribution.new
-        pd.send(:adjustment_on, line_item).should be_nil
+        pd.send(:adjustment_for, line_item).should be_nil
       end
 
       it "returns the adjustment when present" do
         pd = create(:product_distribution)
         line_item = create(:line_item)
-        adjustment = pd.enterprise_fee.create_adjustment('foo', line_item, line_item, true)
+        adjustment = pd.enterprise_fee.create_adjustment('foo', line_item.order, line_item, true)
 
-        pd.send(:adjustment_on, line_item).should == adjustment
+        pd.send(:adjustment_for, line_item).should == adjustment
       end
 
       it "raises an error when there are multiple adjustments for this enterprise fee" do
         pd = create(:product_distribution)
         line_item = create(:line_item)
-        pd.enterprise_fee.create_adjustment('one', line_item, line_item, true)
-        pd.enterprise_fee.create_adjustment('two', line_item, line_item, true)
+        pd.enterprise_fee.create_adjustment('one', line_item.order, line_item, true)
+        pd.enterprise_fee.create_adjustment('two', line_item.order, line_item, true)
 
         expect do
-          pd.send(:adjustment_on, line_item)
+          pd.send(:adjustment_for, line_item)
         end.to raise_error "Multiple adjustments for this enterprise fee on this line item. This method is not designed to deal with this scenario."
       end
     end
 
-    describe "creating an adjustment on a line item" do
+    describe "creating an adjustment for a line item" do
       it "creates the adjustment via the enterprise fee" do
         pd = create(:product_distribution)
-        pd.stub(:adjustment_label) { 'label' }
+        pd.stub(:adjustment_label_for) { 'label' }
         line_item = create(:line_item)
 
-        expect { pd.send(:create_adjustment_on, line_item) }.to change(Spree::Adjustment, :count).by(1)
+        expect { pd.send(:create_adjustment_for, line_item) }.to change(Spree::Adjustment, :count).by(1)
 
         adjustment = Spree::Adjustment.last
         adjustment.label.should == 'label'
-        adjustment.adjustable.should == line_item
+        adjustment.adjustable.should == line_item.order
         adjustment.source.should == line_item
         adjustment.originator.should == pd.enterprise_fee
         adjustment.should be_mandatory
       end
     end
 
-    describe "clearing all enterprise fee adjustments on a line item" do
+    describe "clearing all enterprise fee adjustments for a line item" do
       it "clears adjustments originating from many different enterprise fees" do
         p = create(:simple_product)
         d1, d2 = create(:distributor_enterprise), create(:distributor_enterprise)
         pd1 = create(:product_distribution, product: p, distributor: d1)
         pd2 = create(:product_distribution, product: p, distributor: d2)
         line_item = create(:line_item, product: p)
-        pd1.enterprise_fee.create_adjustment('foo1', line_item, line_item, true)
-        pd2.enterprise_fee.create_adjustment('foo2', line_item, line_item, true)
+        pd1.enterprise_fee.create_adjustment('foo1', line_item.order, line_item, true)
+        pd2.enterprise_fee.create_adjustment('foo2', line_item.order, line_item, true)
 
         expect do
-          pd1.send(:clear_all_enterprise_fee_adjustments_on, line_item)
-        end.to change(line_item.adjustments, :count).by(-2)
+          pd1.send(:clear_all_enterprise_fee_adjustments_for, line_item)
+        end.to change(line_item.order.adjustments, :count).by(-2)
       end
 
       it "does not clear adjustments originating from another source" do
@@ -170,12 +175,20 @@ describe ProductDistribution do
         pd = create(:product_distribution)
         line_item = create(:line_item, product: pd.product)
         tax_rate = create(:tax_rate, calculator: build(:calculator, preferred_amount: 10))
-        tax_rate.create_adjustment('foo', line_item, line_item)
+        tax_rate.create_adjustment('foo', line_item.order, line_item)
 
         expect do
-          pd.send(:clear_all_enterprise_fee_adjustments_on, line_item)
-        end.to change(line_item.adjustments, :count).by(0)
+          pd.send(:clear_all_enterprise_fee_adjustments_for, line_item)
+        end.to change(line_item.order.adjustments, :count).by(0)
       end
     end
   end
+
+
+  private
+
+  def fire_order_contents_changed_event(user, order)
+    ActiveSupport::Notifications.instrument('spree.order.contents_changed', {user: user, order: order})
+  end
+
 end

From b84c476348710940ebd7d3880a6c4b82b7585616 Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Fri, 9 Aug 2013 14:40:37 +1000
Subject: [PATCH 7/8] Upgrade Rails to 3.2.14, use forked spree with
 calculators that are compatible with LineItems

---
 Gemfile      |  4 +--
 Gemfile.lock | 86 ++++++++++++++++++++++++++--------------------------
 2 files changed, 45 insertions(+), 45 deletions(-)

diff --git a/Gemfile b/Gemfile
index 5fef02966a..f7de9999e1 100644
--- a/Gemfile
+++ b/Gemfile
@@ -1,10 +1,10 @@
 source 'https://rubygems.org'
 ruby "1.9.3"
 
-gem 'rails', '3.2.13'
+gem 'rails', '3.2.14'
 
 gem 'pg'
-gem 'spree', :git => 'git://github.com/spree/spree.git', :branch => '1-3-stable'
+gem 'spree', :github => 'eaterprises/spree', :branch => '1-3-stable'
 gem 'spree_i18n', :git => 'git://github.com/spree/spree_i18n.git'
 gem 'spree_paypal_express', :git => 'git://github.com/spree/spree_paypal_express.git', :branch => '1-3-stable'
 gem 'spree_last_address', :git => 'git://github.com/eaterprises/spree-last-address.git', :branch => '1-3-stable'
diff --git a/Gemfile.lock b/Gemfile.lock
index f070db7595..1111a09f7d 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -23,17 +23,8 @@ GIT
       spree_core (>= 1.1)
 
 GIT
-  remote: git://github.com/spree/deface.git
-  revision: 1110a1336252109bce7f98f9182042e0bc2930ae
-  specs:
-    deface (1.0.0.rc3)
-      colorize (>= 0.5.8)
-      nokogiri (~> 1.6.0)
-      rails (>= 3.1)
-
-GIT
-  remote: git://github.com/spree/spree.git
-  revision: 94566bf1b7d50f474333d10844534e570b6edf5a
+  remote: git://github.com/eaterprises/spree.git
+  revision: 4fa1f51ba70519bfd3be0975e5b71f2ea73bbbd6
   branch: 1-3-stable
   specs:
     spree (1.3.3)
@@ -48,7 +39,7 @@ GIT
     spree_cmd (1.3.3)
       thor (>= 0.14.6)
     spree_core (1.3.3)
-      activemerchant (~> 1.31)
+      activemerchant (~> 1.34)
       acts_as_list (= 0.1.4)
       awesome_nested_set (= 2.1.5)
       aws-sdk (~> 1.3.4)
@@ -62,7 +53,7 @@ GIT
       money (= 5.0.0)
       paperclip (~> 2.8)
       rabl (= 0.7.2)
-      rails (~> 3.2.13)
+      rails (~> 3.2.14)
       ransack (= 0.7.2)
       select2-rails (= 3.2.1)
       state_machine (= 1.1.2)
@@ -73,6 +64,15 @@ GIT
     spree_sample (1.3.3)
       spree_core (= 1.3.3)
 
+GIT
+  remote: git://github.com/spree/deface.git
+  revision: 1110a1336252109bce7f98f9182042e0bc2930ae
+  specs:
+    deface (1.0.0.rc3)
+      colorize (>= 0.5.8)
+      nokogiri (~> 1.6.0)
+      rails (>= 3.1)
+
 GIT
   remote: git://github.com/spree/spree_i18n.git
   revision: a96bee02340e008e60549295a4f09e047fd2e628
@@ -124,12 +124,12 @@ PATH
 GEM
   remote: https://rubygems.org/
   specs:
-    actionmailer (3.2.13)
-      actionpack (= 3.2.13)
-      mail (~> 2.5.3)
-    actionpack (3.2.13)
-      activemodel (= 3.2.13)
-      activesupport (= 3.2.13)
+    actionmailer (3.2.14)
+      actionpack (= 3.2.14)
+      mail (~> 2.5.4)
+    actionpack (3.2.14)
+      activemodel (= 3.2.14)
+      activesupport (= 3.2.14)
       builder (~> 3.0.0)
       erubis (~> 2.7.0)
       journey (~> 1.0.4)
@@ -138,7 +138,7 @@ GEM
       rack-test (~> 0.6.1)
       sprockets (~> 2.2.1)
     active_link_to (1.0.0)
-    active_utils (1.0.5)
+    active_utils (2.0.0)
       activesupport (>= 2.3.11)
       i18n
     activemerchant (1.34.0)
@@ -149,19 +149,19 @@ GEM
       json (>= 1.5.1)
       money
       nokogiri
-    activemodel (3.2.13)
-      activesupport (= 3.2.13)
+    activemodel (3.2.14)
+      activesupport (= 3.2.14)
       builder (~> 3.0.0)
-    activerecord (3.2.13)
-      activemodel (= 3.2.13)
-      activesupport (= 3.2.13)
+    activerecord (3.2.14)
+      activemodel (= 3.2.14)
+      activesupport (= 3.2.14)
       arel (~> 3.0.2)
       tzinfo (~> 0.3.29)
-    activeresource (3.2.13)
-      activemodel (= 3.2.13)
-      activesupport (= 3.2.13)
-    activesupport (3.2.13)
-      i18n (= 0.6.1)
+    activeresource (3.2.14)
+      activemodel (= 3.2.14)
+      activesupport (= 3.2.14)
+    activesupport (3.2.14)
+      i18n (~> 0.6, >= 0.6.4)
       multi_json (~> 1.0)
     acts_as_list (0.1.4)
     addressable (2.3.3)
@@ -272,7 +272,7 @@ GEM
     httparty (0.11.0)
       multi_json (~> 1.0)
       multi_xml (>= 0.5.2)
-    i18n (0.6.1)
+    i18n (0.6.4)
     journey (1.0.4)
     jquery-rails (2.2.2)
       railties (>= 3.0, < 5.0)
@@ -298,7 +298,7 @@ GEM
       i18n (~> 0.4)
       json
     multi_json (1.7.8)
-    multi_xml (0.5.4)
+    multi_xml (0.5.5)
     net-scp (1.1.2)
       net-ssh (>= 2.6.5)
     net-ssh (2.6.8)
@@ -337,17 +337,17 @@ GEM
       rack
     rack-test (0.6.2)
       rack (>= 1.0)
-    rails (3.2.13)
-      actionmailer (= 3.2.13)
-      actionpack (= 3.2.13)
-      activerecord (= 3.2.13)
-      activeresource (= 3.2.13)
-      activesupport (= 3.2.13)
+    rails (3.2.14)
+      actionmailer (= 3.2.14)
+      actionpack (= 3.2.14)
+      activerecord (= 3.2.14)
+      activeresource (= 3.2.14)
+      activesupport (= 3.2.14)
       bundler (~> 1.0)
-      railties (= 3.2.13)
-    railties (3.2.13)
-      actionpack (= 3.2.13)
-      activesupport (= 3.2.13)
+      railties (= 3.2.14)
+    railties (3.2.14)
+      actionpack (= 3.2.14)
+      activesupport (= 3.2.14)
       rack-ssl (~> 1.3.2)
       rake (>= 0.8.7)
       rdoc (~> 3.4)
@@ -476,7 +476,7 @@ DEPENDENCIES
   poltergeist
   pry-debugger
   rabl
-  rails (= 3.2.13)
+  rails (= 3.2.14)
   representative_view
   rspec-rails
   sass

From b065d7db3661eb859523cae3dd09fe39d2c5b1c4 Mon Sep 17 00:00:00 2001
From: Rohan Mitchell <rohan@rohanmitchell.com>
Date: Fri, 9 Aug 2013 15:25:50 +1000
Subject: [PATCH 8/8] When updating product distribution charge, skip line
 items that don't have a product distribution

---
 app/models/spree/order_decorator.rb | 2 +-
 spec/models/order_spec.rb           | 9 +++++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb
index 290fb45a83..7a01b408bb 100644
--- a/app/models/spree/order_decorator.rb
+++ b/app/models/spree/order_decorator.rb
@@ -64,7 +64,7 @@ Spree::Order.class_eval do
   def update_distribution_charge!
     line_items.each do |line_item|
       pd = product_distribution_for line_item
-      pd.ensure_correct_adjustment_for line_item
+      pd.ensure_correct_adjustment_for line_item if pd
     end
   end
 
diff --git a/spec/models/order_spec.rb b/spec/models/order_spec.rb
index eb3a968a5c..2cb7dc97e8 100644
--- a/spec/models/order_spec.rb
+++ b/spec/models/order_spec.rb
@@ -42,6 +42,15 @@ describe Spree::Order do
       subject.send(:update_distribution_charge!)
     end
 
+    it "skips line items that don't have a product distribution" do
+      line_item = double(:line_item)
+      subject.stub(:line_items) { [line_item] }
+
+      subject.stub(:product_distribution_for) { nil }
+
+      subject.send(:update_distribution_charge!)
+    end
+
     it "looks up product distribution enterprise fees for a line item" do
       product = double(:product)
       variant = double(:variant, product: product)