Merge branch 'master' of github.com:eaterprises/openfoodweb

Conflicts:
	db/schema.rb

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

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
 

app/models/enterprise_fee.rb

@@ -2,7 +2,6 @@ class EnterpriseFee < ActiveRecord::Base
   belongs_to :enterprise
 
   calculated_adjustments
-  has_one   :calculator, :as => :calculable, :dependent => :destroy, :class_name => 'Spree::Calculator'
 
   attr_accessible :enterprise_id, :fee_type, :name, :calculator_type
 

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

app/models/product_distribution.rb

@@ -2,6 +2,7 @@ class ProductDistribution < ActiveRecord::Base
   belongs_to :product, :class_name => 'Spree::Product'
   belongs_to :distributor, :class_name => 'Enterprise'
   belongs_to :shipping_method, :class_name => 'Spree::ShippingMethod'
+  belongs_to :enterprise_fee
 
   validates_presence_of :product_id, :on => :update
   validates_presence_of :distributor_id, :shipping_method_id

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

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

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

db/migrate/20130805050109_update_line_item_caching.rb

@@ -0,0 +1,41 @@
+class UpdateLineItemCaching < ActiveRecord::Migration
+
+  class SpreeLineItem < ActiveRecord::Base
+    belongs_to :shipping_method, class_name: 'Spree::ShippingMethod'
+
+    def itemwise_shipping_cost
+      order = OpenStruct.new :line_items => [self]
+      shipping_method.compute_amount(order)
+    end
+  end
+
+
+  def up
+    add_column :spree_line_items, :distribution_fee, :decimal, precision: 10, scale: 2
+    add_column :spree_line_items, :shipping_method_name, :string
+
+    SpreeLineItem.all.each do |line_item|
+      line_item.update_column(:distribution_fee, line_item.itemwise_shipping_cost)
+      line_item.update_column(:shipping_method_name, line_item.shipping_method.name)
+    end
+
+    remove_column :spree_line_items, :shipping_method_id
+  end
+
+  def down
+    add_column :spree_line_items, :shipping_method_id, :integer
+
+    SpreeLineItem.all.each do |line_item|
+      shipping_method = Spree::ShippingMethod.find_by_name(line_item.shipping_method_name)
+      unless shipping_method
+        say "Shipping method #{line_item.shipping_method_name} not found, using the first available shipping method for LineItem #{line_item.id}"
+        shipping_method = Spree::ShippingMethod.where("name != 'Delivery'").first
+      end
+
+      line_item.update_column(:shipping_method_id, shipping_method.id)
+    end
+
+    remove_column :spree_line_items, :distribution_fee
+    remove_column :spree_line_items, :shipping_method_name
+  end
+end

db/migrate/20130805232516_add_enterprise_fee_to_product_distributions.rb

@@ -0,0 +1,5 @@
+class AddEnterpriseFeeToProductDistributions < ActiveRecord::Migration
+  def change
+    add_column :product_distributions, :enterprise_fee_id, :integer
+  end
+end

db/schema.rb

@@ -218,6 +218,7 @@ ActiveRecord::Schema.define(:version => 20130806055125) do
     t.integer  "shipping_method_id"
     t.datetime "created_at"
     t.datetime "updated_at"
+    t.integer  "enterprise_fee_id"
   end
 
   create_table "spree_activators", :force => true do |t|
@@ -364,13 +365,14 @@ ActiveRecord::Schema.define(:version => 20130806055125) do
   create_table "spree_line_items", :force => true do |t|
     t.integer  "order_id"
     t.integer  "variant_id"
-    t.integer  "quantity",                                         :null => false
-    t.decimal  "price",              :precision => 8, :scale => 2, :null => false
-    t.datetime "created_at",                                       :null => false
-    t.datetime "updated_at",                                       :null => false
+    t.integer  "quantity",                                            :null => false
+    t.decimal  "price",                :precision => 8,  :scale => 2, :null => false
+    t.datetime "created_at",                                          :null => false
+    t.datetime "updated_at",                                          :null => false
     t.integer  "max_quantity"
-    t.integer  "shipping_method_id"
     t.string   "currency"
+    t.decimal  "distribution_fee",     :precision => 10, :scale => 2
+    t.string   "shipping_method_name"
   end
 
   add_index "spree_line_items", ["order_id"], :name => "index_line_items_on_order_id"

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

spec/factories.rb

@@ -110,6 +110,7 @@ FactoryGirl.define do
     after(:create) do |order|
       p = create(:simple_product, :distributors => [order.distributor])
       FactoryGirl.create(:line_item, :order => order, :product => p)
+      order.reload
     end
   end
 end
@@ -139,10 +140,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

spec/features/admin/order_spec.rb

@@ -12,13 +12,15 @@ feature %q{
     @order = create(:order_with_totals_and_distributor, :user => @user, :state => 'complete', :payment_state => 'balance_due')
 
     # ensure order has a payment to capture
-    create :check_payment, order: @order, amount: @order.amount
     @order.finalize!
+
+    create :check_payment, order: @order, amount: @order.total
   end
 
   context "managing orders" do
     scenario "capture multiple payments from the orders index page" do
       # d.cook: could also test for an order that has had payment voided, then a new check payment created but not yet captured. But it's not critical and I know it works anyway.
+
       login_to_admin_section
 
       click_link 'Orders'

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

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

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

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)

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