diff --git a/Gemfile b/Gemfile index 943aef58be..921cea1f52 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 748677589e..c30add0ad8 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) @@ -479,7 +479,7 @@ DEPENDENCIES pry-debugger rabl rack-ssl - rails (= 3.2.13) + rails (= 3.2.14) representative_view rspec-rails sass 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/app/models/product_distribution.rb b/app/models/product_distribution.rb index eefde4dd63..7ef2a90cdf 100644 --- a/app/models/product_distribution.rb +++ b/app/models/product_distribution.rb @@ -7,4 +7,33 @@ 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 + clear_all_enterprise_fee_adjustments_for line_item + create_adjustment_for line_item + end + end + + 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_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_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_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 a5f5f87c0d..7a01b408bb 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' @@ -22,6 +26,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 +61,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 if pd + end + end + def set_variant_attributes(variant, attributes) line_item = find_line_item_by_variant(variant) @@ -113,4 +125,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/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/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/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 diff --git a/spec/models/order_spec.rb b/spec/models/order_spec.rb index f9dca32470..2cb7dc97e8 100644 --- a/spec/models/order_spec.rb +++ b/spec/models/order_spec.rb @@ -28,6 +28,41 @@ describe Spree::Order do li.max_quantity.should == 3 end + describe "updating the distribution charge" do + let(:order) { build(:order) } + + 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 "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) + 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..a1f92d2cb3 100644 --- a/spec/models/product_distribution_spec.rb +++ b/spec/models/product_distribution_spec.rb @@ -20,4 +20,175 @@ 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, name: 'Pear') + 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 + + # 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 + 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} for Pear" + 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(: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_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_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) + pd = ProductDistribution.new + 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.order, line_item, true) + + 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.order, line_item, true) + pd.enterprise_fee.create_adjustment('two', line_item.order, line_item, true) + + expect do + 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 for a line item" do + it "creates the adjustment via the enterprise fee" do + pd = create(:product_distribution) + pd.stub(:adjustment_label_for) { 'label' } + line_item = create(:line_item) + + 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.order + adjustment.source.should == line_item + adjustment.originator.should == pd.enterprise_fee + adjustment.should be_mandatory + end + end + + 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.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_for, line_item) + end.to change(line_item.order.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.order, line_item) + + expect do + 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 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)