Adjustments on LineItems don't modify the order total. Make adjustments on Order instead.

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

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

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')

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] }

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