diff --git a/app/controllers/spree/admin/shipping_methods_controller_decorator.rb b/app/controllers/spree/admin/shipping_methods_controller_decorator.rb index 84af3ef896..9077c7a41e 100644 --- a/app/controllers/spree/admin/shipping_methods_controller_decorator.rb +++ b/app/controllers/spree/admin/shipping_methods_controller_decorator.rb @@ -16,12 +16,6 @@ module Spree flash[:error] = "That shipping method cannot be deleted as it is referenced by a product distribution: #{p.id} - #{p.name}." redirect_to collection_url and return end - - line_item = LineItem.where(:shipping_method_id => @object).first - if line_item - flash[:error] = "That shipping method cannot be deleted as it is referenced by a line item in order: #{line_item.order.number}." - redirect_to collection_url and return - end end end end diff --git a/app/helpers/spree/orders_helper.rb b/app/helpers/spree/orders_helper.rb index 09faba61ee..745d95cf02 100644 --- a/app/helpers/spree/orders_helper.rb +++ b/app/helpers/spree/orders_helper.rb @@ -7,7 +7,7 @@ module Spree def order_delivery_fee_subtotal(order, options={}) options.reverse_merge! :format_as_currency => true - amount = order.line_items.map { |li| li.itemwise_shipping_cost }.sum + amount = OpenFoodWeb::Calculator::Itemwise.new.compute(order) options.delete(:format_as_currency) ? number_to_currency(amount) : amount end diff --git a/app/models/open_food_web/calculator/itemwise.rb b/app/models/open_food_web/calculator/itemwise.rb index 5a955a639a..76f54cc62c 100644 --- a/app/models/open_food_web/calculator/itemwise.rb +++ b/app/models/open_food_web/calculator/itemwise.rb @@ -7,7 +7,7 @@ module OpenFoodWeb def compute(object) # Given an order, sum the shipping on each individual item - object.line_items.map { |li| li.itemwise_shipping_cost }.inject(:+) || 0 + object.line_items.sum { |li| li.distribution_fee } || 0 end end end diff --git a/app/models/spree/line_item_decorator.rb b/app/models/spree/line_item_decorator.rb index ec4f83e5e2..edca66c6fb 100644 --- a/app/models/spree/line_item_decorator.rb +++ b/app/models/spree/line_item_decorator.rb @@ -1,24 +1,28 @@ Spree::LineItem.class_eval do - belongs_to :shipping_method - attr_accessible :max_quantity - before_create :set_itemwise_shipping_method + before_create :set_distribution_fee - def itemwise_shipping_cost - order = OpenStruct.new :line_items => [self] - shipping_method.compute_amount(order) - end - - def update_itemwise_shipping_method_without_callbacks!(distributor) - update_column(:shipping_method_id, self.product.shipping_method_for_distributor(distributor).id) + def update_distribution_fee_without_callbacks!(distributor) + set_distribution_fee(distributor) + update_column(:distribution_fee, distribution_fee) + update_column(:shipping_method_name, shipping_method_name) end private - def set_itemwise_shipping_method - self.shipping_method = self.product.shipping_method_for_distributor(self.order.distributor) + def shipping_method(distributor=nil) + distributor ||= self.order.distributor + self.product.shipping_method_for_distributor(distributor) + end + + def set_distribution_fee(distributor=nil) + order = OpenStruct.new :line_items => [self] + sm = shipping_method(distributor) + + self.distribution_fee = sm.compute_amount(order) + self.shipping_method_name = sm.name end end diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index b1dba6cec3..0ab43efe51 100644 --- a/app/models/spree/order_decorator.rb +++ b/app/models/spree/order_decorator.rb @@ -99,7 +99,7 @@ Spree::Order.class_eval do def update_line_item_shipping_methods if %w(cart address delivery resumed).include? state - self.line_items.each { |li| li.update_itemwise_shipping_method_without_callbacks!(distributor) } + self.line_items.each { |li| li.update_distribution_fee_without_callbacks!(distributor) } self.update! end end diff --git a/app/views/spree/orders/_distributor_fees.html.haml b/app/views/spree/orders/_distributor_fees.html.haml index ff3e640c2d..015d4d77ee 100644 --- a/app/views/spree/orders/_distributor_fees.html.haml +++ b/app/views/spree/orders/_distributor_fees.html.haml @@ -14,8 +14,8 @@ %tr{:class => tr_class} %td %h4= link_to line_item.product.name, product_path(line_item.product) - %td.item-shipping-method= line_item.shipping_method.name - %td.item-shipping-cost= number_to_currency line_item.itemwise_shipping_cost + %td.item-shipping-method= line_item.shipping_method_name + %td.item-shipping-cost= number_to_currency line_item.distribution_fee .subtotal{'data-hook' => 'delivery_fees_subtotal'} %h5 diff --git a/lib/open_food_web/order_and_distributor_report.rb b/lib/open_food_web/order_and_distributor_report.rb index bcd7aba08d..04a3596441 100644 --- a/lib/open_food_web/order_and_distributor_report.rb +++ b/lib/open_food_web/order_and_distributor_report.rb @@ -20,7 +20,7 @@ module OpenFoodWeb order.line_items.each do |line_item| order_and_distributor_details << [order.created_at, order.id, order.bill_address.full_name, order.email, order.bill_address.phone, order.bill_address.city, - line_item.product.sku, line_item.product.name, line_item.variant.options_text, line_item.quantity, line_item.max_quantity, line_item.price * line_item.quantity, line_item.itemwise_shipping_cost, + line_item.product.sku, line_item.product.name, line_item.variant.options_text, line_item.quantity, line_item.max_quantity, line_item.price * line_item.quantity, line_item.distribution_fee, order.payments.first.payment_method.andand.name, order.distributor.andand.name, order.distributor.address.address1, order.distributor.address.city, order.distributor.address.zipcode, order.special_instructions ] end diff --git a/spec/factories.rb b/spec/factories.rb index 84140468d1..15fa2a80b7 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -139,10 +139,6 @@ FactoryGirl.modify do end end - factory :line_item do - shipping_method { |li| li.product.shipping_method_for_distributor(li.order.distributor) } - end - factory :shipping_method do display_on '' end diff --git a/spec/features/admin/shipping_methods_spec.rb b/spec/features/admin/shipping_methods_spec.rb index baa1b6757e..9f4f1f9490 100644 --- a/spec/features/admin/shipping_methods_spec.rb +++ b/spec/features/admin/shipping_methods_spec.rb @@ -37,24 +37,4 @@ feature 'shipping methods' do page.should have_content "That shipping method cannot be deleted as it is referenced by a product distribution: #{p.id} - #{p.name}." Spree::ShippingMethod.find(@sm.id).should_not be_nil end - - scenario "deleting a shipping method referenced by a line item" do - sm2 = create(:shipping_method) - d = create(:distributor_enterprise) - - p = create(:product) - create(:product_distribution, product: p, distributor: d, shipping_method: sm2) - - o = create(:order, distributor: d) - o.shipping_method = sm2 - o.save! - li = create(:line_item, order: o, product: p) - li.shipping_method = @sm - li.save! - - visit_delete spree.admin_shipping_method_path(@sm) - - page.should have_content "That shipping method cannot be deleted as it is referenced by a line item in order: #{o.number}." - Spree::ShippingMethod.find(@sm.id).should_not be_nil - end end diff --git a/spec/features/consumer/add_to_cart_spec.rb b/spec/features/consumer/add_to_cart_spec.rb index 5423a0ce99..c5e01c1efb 100644 --- a/spec/features/consumer/add_to_cart_spec.rb +++ b/spec/features/consumer/add_to_cart_spec.rb @@ -54,11 +54,12 @@ feature %q{ page.should have_selector 'span.shipping-total', :text => '$1.23' page.should have_selector 'span.grand-total', :text => '$13.57' - # And the item should be in my cart, with shipping method set for the line item + # And the item should be in my cart, with the distribution info set for the line item order = Spree::Order.last line_item = order.line_items.first line_item.product.should == p - line_item.shipping_method.should == p.product_distributions.first.shipping_method + line_item.distribution_fee.should == 1.23 + line_item.shipping_method_name.should == p.product_distributions.first.shipping_method.name # And my order should have its distributor set to the chosen distributor order.distributor.should == d1 diff --git a/spec/lib/open_food_web/order_and_distributor_report_spec.rb b/spec/lib/open_food_web/order_and_distributor_report_spec.rb index 8081319391..cabeb5010f 100644 --- a/spec/lib/open_food_web/order_and_distributor_report_spec.rb +++ b/spec/lib/open_food_web/order_and_distributor_report_spec.rb @@ -39,7 +39,7 @@ module OpenFoodWeb table[0].should == [@order.created_at, @order.id, @bill_address.full_name, @order.email, @bill_address.phone, @bill_address.city, - @line_item.product.sku, @line_item.product.name, @line_item.variant.options_text, @line_item.quantity, @line_item.max_quantity, @line_item.price * @line_item.quantity, @line_item.itemwise_shipping_cost, + @line_item.product.sku, @line_item.product.name, @line_item.variant.options_text, @line_item.quantity, @line_item.max_quantity, @line_item.price * @line_item.quantity, @line_item.distribution_fee, @payment_method.name, @distributor.name, @distributor.address.address1, @distributor.address.city, @distributor.address.zipcode, @shipping_instructions ] end diff --git a/spec/models/calculator/itemwise_spec.rb b/spec/models/calculator/itemwise_spec.rb index 98677d6106..781a285e80 100644 --- a/spec/models/calculator/itemwise_spec.rb +++ b/spec/models/calculator/itemwise_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe OpenFoodWeb::Calculator::Itemwise do it "computes the shipping cost for each line item in an order" do line_item = double(:line_item) - line_item.should_receive(:itemwise_shipping_cost).exactly(3).times.and_return(10) + line_item.should_receive(:distribution_fee).exactly(3).times.and_return(10) order = double(:order) order.stub(:line_items).and_return([line_item]*3) diff --git a/spec/models/line_item_spec.rb b/spec/models/line_item_spec.rb index c447d4eb64..2b46b7d7f7 100644 --- a/spec/models/line_item_spec.rb +++ b/spec/models/line_item_spec.rb @@ -3,21 +3,51 @@ require 'spec_helper' module Spree describe LineItem do describe "computing shipping cost for its product" do - let(:shipping_method) do - sm = create(:shipping_method) + let(:shipping_method_10) do + sm = create(:shipping_method, name: 'SM10') sm.calculator.set_preference :amount, 10 sm end - let(:order) { double(:order, :distributor => nil, :state => 'complete') } - let(:line_item) do - li = LineItem.new - li.stub(:shipping_method).and_return(shipping_method) - li.stub(:order).and_return(order) - li + let(:shipping_method_20) do + sm = create(:shipping_method, name: 'SM20') + sm.calculator.set_preference :amount, 20 + sm + end + let(:distributor1) { create(:distributor_enterprise) } + let(:distributor2) { create(:distributor_enterprise) } + let!(:product) do + p = create(:product) + create(:product_distribution, product: p, distributor: distributor1, shipping_method: shipping_method_10) + create(:product_distribution, product: p, distributor: distributor2, shipping_method: shipping_method_20) + p + end + let(:order_d1) { create(:order, distributor: distributor1, state: 'complete') } + + it "sets distribution fee and shipping method name on creation" do + li = create(:line_item, order: order_d1, product: product) + li.distribution_fee.should == 10 + li.shipping_method_name.should == 'SM10' end - it "computes shipping cost for its product" do - line_item.itemwise_shipping_cost.should == 10 + it "updates its distribution fee & shipping method name" do + li = create(:line_item, order: order_d1, product: product) + + li.update_distribution_fee_without_callbacks! distributor2 + + li.distribution_fee.should == 20 + li.shipping_method_name.should == 'SM20' + end + + describe "fetching its shipping method" do + it "fetches the shipping method for its product when distributor is supplied" do + li = create(:line_item, order: order_d1, product: product) + li.send(:shipping_method, distributor2).should == shipping_method_20 + end + + it "uses the order's distributor when no other distributor is provided" do + li = create(:line_item, order: order_d1, product: product) + li.send(:shipping_method).should == shipping_method_10 + end end end end