diff --git a/app/models/enterprise_fee.rb b/app/models/enterprise_fee.rb index 9be96e853b..ad13a36c7a 100644 --- a/app/models/enterprise_fee.rb +++ b/app/models/enterprise_fee.rb @@ -17,4 +17,9 @@ class EnterpriseFee < ActiveRecord::Base def self.clear_all_adjustments_for(line_item) line_item.order.adjustments.where(originator_type: 'EnterpriseFee', source_id: line_item, source_type: 'Spree::LineItem').destroy_all end + + def self.clear_all_adjustments_on_order(order) + order.adjustments.where(originator_type: 'EnterpriseFee', source_type: 'Spree::LineItem').destroy_all + end + end diff --git a/app/models/order_cycle.rb b/app/models/order_cycle.rb index 3154264948..f8cb0a35a1 100644 --- a/app/models/order_cycle.rb +++ b/app/models/order_cycle.rb @@ -59,19 +59,14 @@ class OrderCycle < ActiveRecord::Base # -- Fees - def ensure_correct_adjustments_for(line_item) - EnterpriseFee.clear_all_adjustments_for line_item - create_adjustments_for line_item + def create_adjustments_for(line_item) + fees_for(line_item).each { |fee| create_adjustment_for_fee line_item, fee[:enterprise_fee], fee[:label], fee[:role] } end private # -- Fees - def create_adjustments_for(line_item) - fees_for(line_item).each { |fee| create_adjustment_for_fee line_item, fee[:enterprise_fee], fee[:label], fee[:role] } - end - def fees_for(line_item) fees = [] diff --git a/app/models/product_distribution.rb b/app/models/product_distribution.rb index d3c4c4a8e9..afc39ca2bf 100644 --- a/app/models/product_distribution.rb +++ b/app/models/product_distribution.rb @@ -8,11 +8,6 @@ class ProductDistribution < ActiveRecord::Base validates_uniqueness_of :product_id, :scope => :distributor_id - def ensure_correct_adjustment_for(line_item) - EnterpriseFee.clear_all_adjustments_for line_item - create_adjustment_for line_item - end - def adjustment_for(line_item) adjustments = line_item.order.adjustments.enterprise_fee.where(originator_id: enterprise_fee) diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index 0058c8db49..f3ae861c84 100644 --- a/app/models/spree/order_decorator.rb +++ b/app/models/spree/order_decorator.rb @@ -54,13 +54,15 @@ Spree::Order.class_eval do end def update_distribution_charge! + EnterpriseFee.clear_all_adjustments_on_order self + line_items.each do |line_item| if provided_by_order_cycle? line_item - order_cycle.ensure_correct_adjustments_for line_item + order_cycle.create_adjustments_for line_item else pd = product_distribution_for line_item - pd.ensure_correct_adjustment_for line_item if pd + pd.create_adjustment_for line_item if pd end end end diff --git a/spec/features/consumer/checkout_spec.rb b/spec/features/consumer/checkout_spec.rb index 2dbc285849..f0a8ec903c 100644 --- a/spec/features/consumer/checkout_spec.rb +++ b/spec/features/consumer/checkout_spec.rb @@ -83,8 +83,11 @@ feature %q{ # Then I should see a breakdown of my delivery fees: table = page.find 'tbody#cart_adjustments' rows = table.all 'tr' - rows[0].all('td').map { |cell| cell.text.strip }.should == ['Product distribution by Edible garden for Fuji apples', '$1.00', ''] - rows[1].all('td').map { |cell| cell.text.strip }.should == ['Product distribution by Edible garden for Garlic', '$2.00', ''] + + rows.map { |row| row.all('td').map { |cell| cell.text.strip } }.should == + [['Product distribution by Edible garden for Fuji apples', '$1.00', ''], + ['Product distribution by Edible garden for Garlic', '$2.00', '']] + page.should have_selector 'span.distribution-total', :text => '$3.00' end @@ -150,6 +153,33 @@ feature %q{ page.should have_content 'Please complete your order from your current order cycle before shopping in a different order cycle.' end + scenario "removing a product from cart removes its fees", js: true do + # Given I am logged in + login_to_consumer_section + + # When I add some apples and some garlic to my cart + click_link 'Fuji apples' + select @distributor.name, :from => 'distributor_id' + click_button 'Add To Cart' + click_link 'Continue shopping' + + click_link 'Garlic' + click_button 'Add To Cart' + + # And I remove the applies + line_item = Spree::Order.last.line_items.first + page.find("a#delete_line_item_#{line_item.id}").click + + # Then I should see fees for only the garlic + table = page.find 'tbody#cart_adjustments' + rows = table.all 'tr' + + rows.map { |row| row.all('td').map { |cell| cell.text.strip } }.should == + [['Product distribution by Edible garden for Garlic', '$2.00', '']] + + page.should have_selector 'span.distribution-total', :text => '$2.00' + end + scenario "changing distributor updates delivery fees" do # Given two distributors and enterprise fees d1 = create(:distributor_enterprise) diff --git a/spec/models/enterprise_fee_spec.rb b/spec/models/enterprise_fee_spec.rb index 4af79ef2ab..98484d6545 100644 --- a/spec/models/enterprise_fee_spec.rb +++ b/spec/models/enterprise_fee_spec.rb @@ -36,4 +36,42 @@ describe EnterpriseFee do end.to change(line_item.order.adjustments, :count).by(0) end end + + describe "clearing all enterprise fee adjustments on an order" do + it "clears adjustments from many fees and one all line items" do + order = create(:order) + + p1 = create(:simple_product) + p2 = create(:simple_product) + d1, d2 = create(:distributor_enterprise), create(:distributor_enterprise) + pd1 = create(:product_distribution, product: p1, distributor: d1) + pd2 = create(:product_distribution, product: p1, distributor: d2) + pd3 = create(:product_distribution, product: p2, distributor: d1) + pd4 = create(:product_distribution, product: p2, distributor: d2) + line_item1 = create(:line_item, order: order, product: p1) + line_item2 = create(:line_item, order: order, product: p2) + pd1.enterprise_fee.create_adjustment('foo1', line_item1.order, line_item1, true) + pd2.enterprise_fee.create_adjustment('foo2', line_item1.order, line_item1, true) + pd3.enterprise_fee.create_adjustment('foo3', line_item2.order, line_item2, true) + pd4.enterprise_fee.create_adjustment('foo4', line_item2.order, line_item2, true) + + expect do + EnterpriseFee.clear_all_adjustments_on_order order + end.to change(order.adjustments, :count).by(-4) + end + + it "does not clear adjustments from another originator" do + order = create(:order) + tax_rate = create(:tax_rate, calculator: stub_model(Spree::Calculator)) + order.adjustments.create({:amount => 12.34, + :source => order, + :originator => tax_rate, + :locked => true, + :label => 'hello' }, :without_protection => true) + + expect do + EnterpriseFee.clear_all_adjustments_on_order order + end.to change(order.adjustments, :count).by(0) + end + end end diff --git a/spec/models/order_cycle_spec.rb b/spec/models/order_cycle_spec.rb index cacc074724..e5a2c58970 100644 --- a/spec/models/order_cycle_spec.rb +++ b/spec/models/order_cycle_spec.rb @@ -141,23 +141,6 @@ describe OrderCycle do end end - describe "ensuring that a line item has the correct adjustment" do - let(:oc) { OrderCycle.new } - let(:line_item) { double(:line_item) } - - it "clears all enterprise fee adjustments on the line item" do - EnterpriseFee.should_receive(:clear_all_adjustments_for).with(line_item) - oc.stub(:create_adjustments_for) - oc.ensure_correct_adjustments_for line_item - end - - it "creates an adjustment on the line item" do - EnterpriseFee.stub(:clear_all_adjustments_for) - oc.should_receive(:create_adjustments_for).with(line_item) - oc.ensure_correct_adjustments_for line_item - end - end - describe "creating adjustments for a line item" do let(:oc) { OrderCycle.new } let(:line_item) { double(:line_item, variant: 123) } diff --git a/spec/models/order_spec.rb b/spec/models/order_spec.rb index 2dcff1d92b..b64ae72eee 100644 --- a/spec/models/order_spec.rb +++ b/spec/models/order_spec.rb @@ -18,38 +18,46 @@ describe Spree::Order do describe "updating the distribution charge" do let(:order) { build(:order) } + it "clears all enterprise fee adjustments on the order" do + EnterpriseFee.should_receive(:clear_all_adjustments_on_order).with(subject) + subject.update_distribution_charge! + end + it "ensures the correct adjustment(s) are created for the product distribution" do + EnterpriseFee.stub(:clear_all_adjustments_on_order) line_item = double(:line_item) subject.stub(:line_items) { [line_item] } subject.stub(:provided_by_order_cycle?) { false } product_distribution = double(:product_distribution) - product_distribution.should_receive(:ensure_correct_adjustment_for).with(line_item) + product_distribution.should_receive(:create_adjustment_for).with(line_item) subject.stub(:product_distribution_for) { product_distribution } - subject.send(:update_distribution_charge!) + subject.update_distribution_charge! end it "skips line items that don't have a product distribution" do + EnterpriseFee.stub(:clear_all_adjustments_on_order) line_item = double(:line_item) subject.stub(:line_items) { [line_item] } subject.stub(:provided_by_order_cycle?) { false } subject.stub(:product_distribution_for) { nil } - subject.send(:update_distribution_charge!) + subject.update_distribution_charge! end it "ensures the correct adjustment(s) are created for order cycles" do + EnterpriseFee.stub(:clear_all_adjustments_on_order) line_item = double(:line_item) subject.stub(:line_items) { [line_item] } subject.stub(:provided_by_order_cycle?) { true } order_cycle = double(:order_cycle) - order_cycle.should_receive(:ensure_correct_adjustments_for).with(line_item) + order_cycle.should_receive(:create_adjustments_for).with(line_item) subject.stub(:order_cycle) { order_cycle } - subject.send(:update_distribution_charge!) + subject.update_distribution_charge! end describe "looking up whether a line item can be provided by an order cycle" do diff --git a/spec/models/product_distribution_spec.rb b/spec/models/product_distribution_spec.rb index 19c15dcdd4..bec4de0350 100644 --- a/spec/models/product_distribution_spec.rb +++ b/spec/models/product_distribution_spec.rb @@ -65,48 +65,6 @@ describe ProductDistribution do 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) } - - describe "adding items to cart" do - it "clears all enterprise fee adjustments on the line item" do - EnterpriseFee.should_receive(:clear_all_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 - EnterpriseFee.stub(:clear_all_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 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.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.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 for a line item" do it "returns nil when not present" do line_item = build(:line_item)