From a8d4efd0672e6c8674ac9e1d8392564d5633a51d Mon Sep 17 00:00:00 2001 From: Andrew Spinks Date: Fri, 16 Aug 2013 16:47:24 +1000 Subject: [PATCH 01/30] Add new 'shop' route that switches to the selected distributor and empties the cart if it has any line_items for a different distributor. --- app/controllers/enterprises_controller.rb | 17 ++++++++- config/routes.rb | 3 +- .../enterprises_controller_spec.rb | 37 +++++++++++++++++++ 3 files changed, 55 insertions(+), 2 deletions(-) diff --git a/app/controllers/enterprises_controller.rb b/app/controllers/enterprises_controller.rb index 772dd88265..9ea4f41a43 100644 --- a/app/controllers/enterprises_controller.rb +++ b/app/controllers/enterprises_controller.rb @@ -1,6 +1,7 @@ include Spree::ProductsHelper + class EnterprisesController < BaseController - + def index @enterprises = Enterprise.all end @@ -32,6 +33,20 @@ class EnterprisesController < BaseController @products = @searcher.retrieve_products end + def shop + distributor = Enterprise.is_distributor.find params[:id] + order = current_order(true) + + if order.distributor and order.distributor != distributor + order.empty! + end + + order.distributor = distributor + order.save! + + redirect_to main_app.enterprise_path(distributor) + end + # essentially the new 'show' action that renders non-spree view # will need to be renamed into show once the old one is removed def shop_front diff --git a/config/routes.rb b/config/routes.rb index 6473fc2385..fbb55fc29f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -9,7 +9,8 @@ Openfoodweb::Application.routes.draw do end member do - get :shop_front + get :shop_front #new world + get :shop # old world end end diff --git a/spec/controllers/enterprises_controller_spec.rb b/spec/controllers/enterprises_controller_spec.rb index 45d0374a84..f728210e2e 100644 --- a/spec/controllers/enterprises_controller_spec.rb +++ b/spec/controllers/enterprises_controller_spec.rb @@ -9,4 +9,41 @@ describe EnterprisesController do assigns(:suppliers).should == [s] end + + context 'shopping for a distributor' do + + before(:each) do + @current_distributor = create(:distributor_enterprise) + @distributor = create(:distributor_enterprise) + controller.current_order(true).distributor = @current_distributor + end + + it "sets the shop as the distributor on the order when shopping for the distributor" do + spree_get :shop, {id: @distributor} + + controller.current_order.distributor.should == @distributor + end + + it "empties an order that was set for a previous distributor, when shopping at a new distributor" do + line_item = create(:line_item) + controller.current_order.line_items << line_item + + spree_get :shop, {id: @distributor} + + controller.current_order.distributor.should == @distributor + controller.current_order.line_items.size.should == 0 + end + + it "should not empty an order if returning to the same distributor" do + product = create(:product) + create(:product_distribution, product: product, distributor: @current_distributor) + line_item = create(:line_item, variant: product.master) + controller.current_order.line_items << line_item + + spree_get :shop, {id: @current_distributor} + + controller.current_order.distributor.should == @current_distributor + controller.current_order.line_items.size.should == 1 + end + end end From 38522e2b74afdaa6b46d52f7ae6d23a8f0513b97 Mon Sep 17 00:00:00 2001 From: Andrew Spinks Date: Sun, 18 Aug 2013 19:41:04 +1000 Subject: [PATCH 02/30] Fix order cycle permissions for enterprise user. --- .../admin/enterprises_controller.rb | 2 +- app/models/spree/ability_decorator.rb | 9 ++++++ spec/features/admin/enterprises_spec.rb | 31 +++++++++++++++++-- spec/models/ability_spec.rb | 21 +++++++++++++ 4 files changed, 60 insertions(+), 3 deletions(-) diff --git a/app/controllers/admin/enterprises_controller.rb b/app/controllers/admin/enterprises_controller.rb index a297eadc9c..981299a8ea 100644 --- a/app/controllers/admin/enterprises_controller.rb +++ b/app/controllers/admin/enterprises_controller.rb @@ -24,7 +24,7 @@ module Admin end def collection - super.order('is_primary_producer DESC, is_distributor ASC, name') + super.managed_by(spree_current_user).order('is_primary_producer DESC, is_distributor ASC, name') end end end diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb index f29b8046c7..34bd9679a4 100644 --- a/app/models/spree/ability_decorator.rb +++ b/app/models/spree/ability_decorator.rb @@ -40,6 +40,15 @@ class AbilityDecorator can [:create], OrderCycle + can [:index, :read], EnterpriseFee + can [:admin, :index, :read, :create, :edit, :update], ExchangeVariant + can [:admin, :index, :read, :create, :edit, :update], Exchange + can [:admin, :index, :read, :create, :edit, :update], ExchangeFee + can [:admin, :index], Enterprise + can [:read, :edit, :update], Enterprise do |enterprise| + user.enterprises.include? enterprise + end + end end end diff --git a/spec/features/admin/enterprises_spec.rb b/spec/features/admin/enterprises_spec.rb index 3f7e72ba88..db97f28b9f 100644 --- a/spec/features/admin/enterprises_spec.rb +++ b/spec/features/admin/enterprises_spec.rb @@ -6,12 +6,12 @@ feature %q{ } do include AuthenticationWorkflow include WebHelper - + before :all do @default_wait_time = Capybara.default_wait_time Capybara.default_wait_time = 5 end - + after :all do Capybara.default_wait_time = @default_wait_time end @@ -127,4 +127,31 @@ feature %q{ Enterprise.is_distributor.map { |d| d.next_collection_at }.should == %w(One Two Three) end + context 'as an Enterprise user' do + + let(:supplier1) { create(:supplier_enterprise, name: 'First Supplier') } + let(:supplier2) { create(:supplier_enterprise, name: 'Another Supplier') } + let(:distributor1) { create(:distributor_enterprise, name: 'First Distributor') } + let(:distributor2) { create(:distributor_enterprise, name: 'Another Distributor') } + + before(:each) do + @new_user = create_enterprise_user + @new_user.enterprise_roles.build(enterprise: supplier1).save + @new_user.enterprise_roles.build(enterprise: distributor1).save + + login_to_admin_as @new_user + end + + scenario "can view enterprises I have permission to" do + oc_user_coordinating = create(:simple_order_cycle, { coordinator: supplier1, name: 'Order Cycle 1' } ) + oc_for_other_user = create(:simple_order_cycle, { coordinator: supplier2, name: 'Order Cycle 2' } ) + + click_link "Enterprises" + + page.should have_content supplier1.name + page.should have_content distributor1.name + page.should_not have_content supplier2.name + page.should_not have_content distributor2.name + end + end end diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb index 9007ff8a2a..0d9bb31cc3 100644 --- a/spec/models/ability_spec.rb +++ b/spec/models/ability_spec.rb @@ -137,6 +137,27 @@ module Spree should have_ability([:create], for: OrderCycle) end end + + context 'Enterprise manager' do + let (:user) do + user = create(:user) + user.spree_roles = [] + s1.enterprise_roles.build(user: user).save + user + end + + it 'should have the ability to read and edit enterprises that I manage' do + should have_ability([:read, :edit, :update], for: s1) + end + + it 'should not have the ability to read and edit enterprises that I do not manage' do + should_not have_ability([:read, :edit, :update], for: s2) + end + + it 'should have the ability administrate enterpises' do + should have_ability([:admin, :index], for: Enterprise) + end + end end end end \ No newline at end of file From 5ffd56aad7e0b3ddb75af66b37d7711d52d0b2e6 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 16 Aug 2013 11:47:04 +1000 Subject: [PATCH 03/30] Re-write spec with new fee display at checkout --- spec/features/consumer/checkout_spec.rb | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/spec/features/consumer/checkout_spec.rb b/spec/features/consumer/checkout_spec.rb index d9220c59aa..b14deda33e 100644 --- a/spec/features/consumer/checkout_spec.rb +++ b/spec/features/consumer/checkout_spec.rb @@ -64,7 +64,7 @@ feature %q{ end - scenario "viewing delivery fees" do + scenario "viewing delivery fees for product distribution" do # Given I am logged in login_to_consumer_section @@ -78,21 +78,16 @@ feature %q{ click_button 'Add To Cart' # Then I should see a breakdown of my delivery fees: - # Item | Shipping Method | Delivery Fee - # Garlic | Shipping Method Two | $2.00 - # Fuji apples | Shipping Method One | $1.00 - # - # Subtotal: $3.00 - - # Disabled pending spec for the new table - # table = page.find 'table#delivery' - # rows = table.all('tr') - # rows[0].all('th').map { |cell| cell.text.strip }.should == ['Item', 'Shipping Method', 'Delivery Fee'] - # rows[1].all('td').map { |cell| cell.text.strip }.should == ['Fuji apples', 'Shipping Method One', '$1.00'] - # rows[2].all('td').map { |cell| cell.text.strip }.should == ['Garlic', 'Shipping Method Two', '$2.00'] - # page.should have_selector '#delivery-fees span.order-total', :text => '$3.00' + 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', ''] + page.should have_selector 'span.distribution-total', :text => '$3.00' end + #scenario "viewing delivery fees for order cycle distribution" + #scenario "viewing delivery fees for mixed product and order cycle distribution" + scenario "changing distributor updates delivery fees" do # Given two distributors and enterprise fees d1 = create(:distributor_enterprise) From e318a1591d415e35962e5c16c0cb79227c225618 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 16 Aug 2013 13:35:30 +1000 Subject: [PATCH 04/30] Add FeatureToggleHelper for tests, enabling feature toggle changes for tests --- spec/spec_helper.rb | 1 + spec/support/feature_toggle_helper.rb | 9 +++++++++ 2 files changed, 10 insertions(+) create mode 100644 spec/support/feature_toggle_helper.rb diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 42d6fc251f..a60cc125a3 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -80,6 +80,7 @@ RSpec.configure do |config| config.include Spree::CheckoutHelpers config.include Spree::Core::TestingSupport::ControllerRequests, :type => :controller config.include Devise::TestHelpers, :type => :controller + config.include OpenFoodWeb::FeatureToggleHelper # Factory girl require 'factory_girl_rails' diff --git a/spec/support/feature_toggle_helper.rb b/spec/support/feature_toggle_helper.rb new file mode 100644 index 0000000000..054bd9fead --- /dev/null +++ b/spec/support/feature_toggle_helper.rb @@ -0,0 +1,9 @@ +module OpenFoodWeb + module FeatureToggleHelper + def set_feature_toggle(feature, status) + features = OpenFoodWeb::FeatureToggle.features + features[feature] = status + OpenFoodWeb::FeatureToggle.stub(:features) { features } + end + end +end From 591f6a8a57cd8d7732b3830f5818262f4c21d959 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 16 Aug 2013 14:10:03 +1000 Subject: [PATCH 05/30] Move ProductDistribution#clear_all_enterprise_fee_adjustments_for to EnterpriseFee class --- app/models/enterprise_fee.rb | 5 ++++ app/models/product_distribution.rb | 6 +---- spec/models/enterprise_fee_spec.rb | 28 +++++++++++++++++++ spec/models/product_distribution_spec.rb | 34 +++--------------------- 4 files changed, 37 insertions(+), 36 deletions(-) diff --git a/app/models/enterprise_fee.rb b/app/models/enterprise_fee.rb index 96b062cdd0..9be96e853b 100644 --- a/app/models/enterprise_fee.rb +++ b/app/models/enterprise_fee.rb @@ -12,4 +12,9 @@ class EnterpriseFee < ActiveRecord::Base scope :for_enterprise, lambda { |enterprise| where(enterprise_id: enterprise) } + + + 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 end diff --git a/app/models/product_distribution.rb b/app/models/product_distribution.rb index fbad4e6dd9..235d369022 100644 --- a/app/models/product_distribution.rb +++ b/app/models/product_distribution.rb @@ -10,7 +10,7 @@ class ProductDistribution < ActiveRecord::Base def ensure_correct_adjustment_for(line_item) if enterprise_fee - clear_all_enterprise_fee_adjustments_for line_item + EnterpriseFee.clear_all_adjustments_for line_item create_adjustment_for line_item end end @@ -28,12 +28,8 @@ class ProductDistribution < ActiveRecord::Base AdjustmentMetadata.create! adjustment: a, enterprise: enterprise_fee.enterprise, fee_name: enterprise_fee.name, fee_type: enterprise_fee.fee_type, enterprise_role: 'distributor' 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/spec/models/enterprise_fee_spec.rb b/spec/models/enterprise_fee_spec.rb index 1616b0decd..4af79ef2ab 100644 --- a/spec/models/enterprise_fee_spec.rb +++ b/spec/models/enterprise_fee_spec.rb @@ -8,4 +8,32 @@ describe EnterpriseFee do describe "validations" do it { should validate_presence_of(:name) } 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 + EnterpriseFee.clear_all_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 + EnterpriseFee.clear_all_adjustments_for line_item + end.to change(line_item.order.adjustments, :count).by(0) + end + end end diff --git a/spec/models/product_distribution_spec.rb b/spec/models/product_distribution_spec.rb index ad18e3f8ad..f7c4c3cbc6 100644 --- a/spec/models/product_distribution_spec.rb +++ b/spec/models/product_distribution_spec.rb @@ -74,20 +74,20 @@ 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(:clear_all_enterprise_fee_adjustments_for).never + EnterpriseFee.should_receive(:clear_all_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) + 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 - pd.stub(:clear_all_enterprise_fee_adjustments_for) + EnterpriseFee.stub(:clear_all_adjustments_for) pd.should_receive(:create_adjustment_for).with(line_item) pd.ensure_correct_adjustment_for line_item end @@ -164,34 +164,6 @@ describe ProductDistribution do md.enterprise_role.should == 'distributor' 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 From 1d23446c4041b827429766b8b5018373b4bad1f6 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 16 Aug 2013 14:12:22 +1000 Subject: [PATCH 06/30] ProductDistribution validates_presence_of enterprise fee, check no longer required --- app/models/product_distribution.rb | 6 ++---- spec/models/product_distribution_spec.rb | 8 -------- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/app/models/product_distribution.rb b/app/models/product_distribution.rb index 235d369022..d3c4c4a8e9 100644 --- a/app/models/product_distribution.rb +++ b/app/models/product_distribution.rb @@ -9,10 +9,8 @@ class ProductDistribution < ActiveRecord::Base def ensure_correct_adjustment_for(line_item) - if enterprise_fee - EnterpriseFee.clear_all_adjustments_for line_item - create_adjustment_for line_item - end + EnterpriseFee.clear_all_adjustments_for line_item + create_adjustment_for line_item end def adjustment_for(line_item) diff --git a/spec/models/product_distribution_spec.rb b/spec/models/product_distribution_spec.rb index f7c4c3cbc6..19c15dcdd4 100644 --- a/spec/models/product_distribution_spec.rb +++ b/spec/models/product_distribution_spec.rb @@ -71,14 +71,6 @@ describe ProductDistribution do 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 - EnterpriseFee.should_receive(:clear_all_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 EnterpriseFee.should_receive(:clear_all_adjustments_for).with(line_item) From 9563aad9fa01ac206b752fa28a0ff594dcd55e32 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 16 Aug 2013 15:12:03 +1000 Subject: [PATCH 07/30] Find exchanges with a particular variant --- app/models/exchange.rb | 1 + spec/models/exchange_spec.rb | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/app/models/exchange.rb b/app/models/exchange.rb index fe7f04933c..8ad87d5302 100644 --- a/app/models/exchange.rb +++ b/app/models/exchange.rb @@ -17,4 +17,5 @@ class Exchange < ActiveRecord::Base scope :incoming, joins(:order_cycle).where('exchanges.receiver_id = order_cycles.coordinator_id') scope :outgoing, joins(:order_cycle).where('exchanges.sender_id = order_cycles.coordinator_id') + scope :with_variant, lambda { |variant| joins(:exchange_variants).where('exchange_variants.variant_id = ?', variant) } end diff --git a/spec/models/exchange_spec.rb b/spec/models/exchange_spec.rb index 523ee23e5e..ff3829ec67 100644 --- a/spec/models/exchange_spec.rb +++ b/spec/models/exchange_spec.rb @@ -60,5 +60,13 @@ describe Exchange do it "finds outgoing exchanges" do Exchange.outgoing.should == [outgoing_exchange] end + + it "finds exchanges with a particular variant" do + v = create(:variant) + ex = create(:exchange) + ex.variants << v + + Exchange.with_variant(v).should == [ex] + end end end From dfd1a89975e3f1d5c11e05563034d8a040430f96 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 16 Aug 2013 15:42:06 +1000 Subject: [PATCH 08/30] Test whether exchanges are incoming --- app/models/exchange.rb | 4 ++++ spec/models/exchange_spec.rb | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/app/models/exchange.rb b/app/models/exchange.rb index 8ad87d5302..a8f9ef9175 100644 --- a/app/models/exchange.rb +++ b/app/models/exchange.rb @@ -18,4 +18,8 @@ class Exchange < ActiveRecord::Base scope :incoming, joins(:order_cycle).where('exchanges.receiver_id = order_cycles.coordinator_id') scope :outgoing, joins(:order_cycle).where('exchanges.sender_id = order_cycles.coordinator_id') scope :with_variant, lambda { |variant| joins(:exchange_variants).where('exchange_variants.variant_id = ?', variant) } + + def incoming? + receiver == order_cycle.coordinator + end end diff --git a/spec/models/exchange_spec.rb b/spec/models/exchange_spec.rb index ff3829ec67..5f342c6ab3 100644 --- a/spec/models/exchange_spec.rb +++ b/spec/models/exchange_spec.rb @@ -44,6 +44,24 @@ describe Exchange do e.enterprise_fees.count.should == 1 end + describe "reporting whether it is an incoming exchange" do + let(:supplier) { create(:supplier_enterprise) } + let(:coordinator) { create(:distributor_enterprise) } + let(:distributor) { create(:distributor_enterprise) } + let(:oc) { create(:simple_order_cycle, coordinator: coordinator) } + + let(:incoming_exchange) { oc.exchanges.create! sender: supplier, receiver: coordinator } + let(:outgoing_exchange) { oc.exchanges.create! sender: coordinator, receiver: distributor } + + it "returns true for incoming exchanges" do + incoming_exchange.should be_incoming + end + + it "returns false for outgoing exchanges" do + outgoing_exchange.should_not be_incoming + end + end + describe "scopes" do let(:supplier) { create(:supplier_enterprise) } let(:coordinator) { create(:distributor_enterprise) } From e15e9a147697b3052b94d8e9005b97435d0c15da Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 19 Aug 2013 09:27:18 +1000 Subject: [PATCH 09/30] Charge order cycle fees --- app/models/order_cycle.rb | 50 ++++++++++++++++++++ app/models/spree/order_decorator.rb | 14 +++++- spec/models/order_cycle_spec.rb | 73 +++++++++++++++++++++++++++++ spec/models/order_spec.rb | 42 +++++++++++++++++ 4 files changed, 177 insertions(+), 2 deletions(-) diff --git a/app/models/order_cycle.rb b/app/models/order_cycle.rb index 51af905060..3ff37bbdf4 100644 --- a/app/models/order_cycle.rb +++ b/app/models/order_cycle.rb @@ -58,4 +58,54 @@ class OrderCycle < ActiveRecord::Base end + # -- Fees + def ensure_correct_adjustments_for(line_item) + EnterpriseFee.clear_all_adjustments_for line_item + create_adjustments_for line_item + 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 = [] + + # If there are multiple distributors with this variant, won't this mean that we get a fee charged for each of them? + # We just want the one matching line_item.order.distributor + + exchanges_carrying(line_item.variant).each do |exchange| + exchange.enterprise_fees.each do |enterprise_fee| + role = exchange.incoming? ? 'supplier' : 'distributor' + fees << {enterprise_fee: enterprise_fee, + label: adjustment_label_for(line_item, enterprise_fee, role), + role: role} + end + end + + coordinator_fees.each do |enterprise_fee| + fees << {enterprise_fee: enterprise_fee, + label: adjustment_label_for(line_item, enterprise_fee, 'coordinator'), + role: 'coordinator'} + end + + fees + end + + def create_adjustment_for_fee(line_item, enterprise_fee, label, role) + a = enterprise_fee.create_adjustment(label, line_item.order, line_item, true) + AdjustmentMetadata.create! adjustment: a, enterprise: enterprise_fee.enterprise, fee_name: enterprise_fee.name, fee_type: enterprise_fee.fee_type, enterprise_role: role + end + + def adjustment_label_for(line_item, enterprise_fee, role) + "#{line_item.variant.product.name} - #{enterprise_fee.fee_type} fee by #{role} #{enterprise_fee.enterprise.name}" + end + + def exchanges_carrying(variant) + exchanges.with_variant(variant) + end end diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index b4d94eb45e..0058c8db49 100644 --- a/app/models/spree/order_decorator.rb +++ b/app/models/spree/order_decorator.rb @@ -55,8 +55,13 @@ Spree::Order.class_eval do 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 + if provided_by_order_cycle? line_item + order_cycle.ensure_correct_adjustments_for line_item + + else + pd = product_distribution_for line_item + pd.ensure_correct_adjustment_for line_item if pd + end end end @@ -96,6 +101,11 @@ Spree::Order.class_eval do end end + def provided_by_order_cycle?(line_item) + order_cycle_variants = order_cycle.andand.variants || [] + order_cycle_variants.include? line_item.variant + end + def product_distribution_for(line_item) line_item.variant.product.product_distribution_for self.distributor end diff --git a/spec/models/order_cycle_spec.rb b/spec/models/order_cycle_spec.rb index 14c17bce04..cacc074724 100644 --- a/spec/models/order_cycle_spec.rb +++ b/spec/models/order_cycle_spec.rb @@ -140,4 +140,77 @@ describe OrderCycle do @oc.products.sort.should == [@p0, @p1, @p2] 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) } + + it "creates adjustment for each fee" do + fee = {enterprise_fee: 'ef', label: 'label', role: 'role'} + oc.stub(:fees_for) { [fee] } + oc.should_receive(:create_adjustment_for_fee).with(line_item, 'ef', 'label', 'role') + + oc.send(:create_adjustments_for, line_item) + end + + it "finds fees for a line item" do + ef1 = double(:enterprise_fee) + ef2 = double(:enterprise_fee) + ef3 = double(:enterprise_fee) + incoming_exchange = double(:exchange, enterprise_fees: [ef1], incoming?: true) + outgoing_exchange = double(:exchange, enterprise_fees: [ef2], incoming?: false) + oc.stub(:exchanges_carrying) { [incoming_exchange, outgoing_exchange] } + oc.stub(:coordinator_fees) { [ef3] } + oc.stub(:adjustment_label_for) { 'label' } + + oc.send(:fees_for, line_item).should == + [{enterprise_fee: ef1, label: 'label', role: 'supplier'}, + {enterprise_fee: ef2, label: 'label', role: 'distributor'}, + {enterprise_fee: ef3, label: 'label', role: 'coordinator'}] + end + + it "creates an adjustment for a fee" do + line_item = create(:line_item) + enterprise_fee = create(:enterprise_fee) + + oc.send(:create_adjustment_for_fee, line_item, enterprise_fee, 'label', 'role') + + adjustment = Spree::Adjustment.last + adjustment.label.should == 'label' + adjustment.adjustable.should == line_item.order + adjustment.source.should == line_item + adjustment.originator.should == enterprise_fee + adjustment.should be_mandatory + + md = adjustment.metadata + md.enterprise.should == enterprise_fee.enterprise + md.fee_name.should == enterprise_fee.name + md.fee_type.should == enterprise_fee.fee_type + md.enterprise_role.should == 'role' + end + + it "makes adjustment labels" do + line_item = double(:line_item, variant: double(:variant, product: double(:product, name: 'Bananas'))) + enterprise_fee = double(:enterprise_fee, fee_type: 'packing', enterprise: double(:enterprise, name: 'Ballantyne')) + + oc.send(:adjustment_label_for, line_item, enterprise_fee, 'distributor').should == "Bananas - packing fee by distributor Ballantyne" + end + end end diff --git a/spec/models/order_spec.rb b/spec/models/order_spec.rb index fe8051ee3c..2dcff1d92b 100644 --- a/spec/models/order_spec.rb +++ b/spec/models/order_spec.rb @@ -21,6 +21,7 @@ describe Spree::Order do it "ensures the correct adjustment(s) are created for the product distribution" do 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) @@ -32,12 +33,53 @@ describe Spree::Order do 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(:provided_by_order_cycle?) { false } subject.stub(:product_distribution_for) { nil } subject.send(:update_distribution_charge!) end + it "ensures the correct adjustment(s) are created for order cycles" do + 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) + subject.stub(:order_cycle) { order_cycle } + + subject.send(:update_distribution_charge!) + end + + describe "looking up whether a line item can be provided by an order cycle" do + it "returns true when the variant is provided" do + v = double(:variant) + line_item = double(:line_item, variant: v) + order_cycle = double(:order_cycle, variants: [v]) + subject.stub(:order_cycle) { order_cycle } + + subject.send(:provided_by_order_cycle?, line_item).should be_true + end + + it "returns false otherwise" do + v = double(:variant) + line_item = double(:line_item, variant: v) + order_cycle = double(:order_cycle, variants: []) + subject.stub(:order_cycle) { order_cycle } + + subject.send(:provided_by_order_cycle?, line_item).should be_false + end + + it "returns false when there is no order cycle" do + v = double(:variant) + line_item = double(:line_item, variant: v) + subject.stub(:order_cycle) { nil } + + subject.send(:provided_by_order_cycle?, line_item).should be_false + end + end + it "looks up product distribution enterprise fees for a line item" do product = double(:product) variant = double(:variant, product: product) From 7a75898b2bcc237e507596344c35c7b5c4253119 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 19 Aug 2013 09:53:08 +1000 Subject: [PATCH 10/30] Find exchanges going to/from any number of enterprises --- app/models/exchange.rb | 2 ++ spec/models/exchange_spec.rb | 10 ++++++++++ 2 files changed, 12 insertions(+) diff --git a/app/models/exchange.rb b/app/models/exchange.rb index a8f9ef9175..b56fef9e8b 100644 --- a/app/models/exchange.rb +++ b/app/models/exchange.rb @@ -17,6 +17,8 @@ class Exchange < ActiveRecord::Base scope :incoming, joins(:order_cycle).where('exchanges.receiver_id = order_cycles.coordinator_id') scope :outgoing, joins(:order_cycle).where('exchanges.sender_id = order_cycles.coordinator_id') + scope :from_enterprises, lambda { |enterprises| where('exchanges.sender_id IN (?)', enterprises) } + scope :to_enterprises, lambda { |enterprises| where('exchanges.receiver_id IN (?)', enterprises) } scope :with_variant, lambda { |variant| joins(:exchange_variants).where('exchange_variants.variant_id = ?', variant) } def incoming? diff --git a/spec/models/exchange_spec.rb b/spec/models/exchange_spec.rb index 5f342c6ab3..3dc72a8a2d 100644 --- a/spec/models/exchange_spec.rb +++ b/spec/models/exchange_spec.rb @@ -79,6 +79,16 @@ describe Exchange do Exchange.outgoing.should == [outgoing_exchange] end + it "finds exchanges going to any of a number of enterprises" do + Exchange.to_enterprises([coordinator]).should == [incoming_exchange] + Exchange.to_enterprises([coordinator, distributor]).should == [incoming_exchange, outgoing_exchange] + end + + it "finds exchanges coming from any of a number of enterprises" do + Exchange.from_enterprises([coordinator]).should == [outgoing_exchange] + Exchange.from_enterprises([supplier, coordinator]).should == [incoming_exchange, outgoing_exchange] + end + it "finds exchanges with a particular variant" do v = create(:variant) ex = create(:exchange) From 80d6e3b87f9ec764bf0b7abe97890919af2ff5ba Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 19 Aug 2013 10:06:26 +1000 Subject: [PATCH 11/30] Do not charge for distributor fees for a distributor you're not checking out with --- app/models/order_cycle.rb | 9 +- spec/factories.rb | 24 ++++-- spec/features/consumer/checkout_spec.rb | 106 +++++++++++++++++++++++- 3 files changed, 127 insertions(+), 12 deletions(-) diff --git a/app/models/order_cycle.rb b/app/models/order_cycle.rb index 3ff37bbdf4..3154264948 100644 --- a/app/models/order_cycle.rb +++ b/app/models/order_cycle.rb @@ -78,7 +78,7 @@ class OrderCycle < ActiveRecord::Base # If there are multiple distributors with this variant, won't this mean that we get a fee charged for each of them? # We just want the one matching line_item.order.distributor - exchanges_carrying(line_item.variant).each do |exchange| + exchanges_carrying(line_item).each do |exchange| exchange.enterprise_fees.each do |enterprise_fee| role = exchange.incoming? ? 'supplier' : 'distributor' fees << {enterprise_fee: enterprise_fee, @@ -105,7 +105,10 @@ class OrderCycle < ActiveRecord::Base "#{line_item.variant.product.name} - #{enterprise_fee.fee_type} fee by #{role} #{enterprise_fee.enterprise.name}" end - def exchanges_carrying(variant) - exchanges.with_variant(variant) + def exchanges_carrying(line_item) + coordinator = line_item.order.order_cycle.coordinator + distributor = line_item.order.distributor + + exchanges.to_enterprises([coordinator, distributor]).with_variant(line_item.variant) end end diff --git a/spec/factories.rb b/spec/factories.rb index 82a8efe333..1a0425f337 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -7,16 +7,22 @@ FactoryGirl.define do after(:create) do |oc| # Suppliers - ex1 = create(:exchange, :order_cycle => oc, :receiver => oc.coordinator) - ex2 = create(:exchange, :order_cycle => oc, :receiver => oc.coordinator) + ex1 = create(:exchange, :order_cycle => oc, + :sender => create(:supplier_enterprise), :receiver => oc.coordinator) + ex2 = create(:exchange, :order_cycle => oc, + :sender => create(:supplier_enterprise), :receiver => oc.coordinator) ExchangeFee.create!(exchange: ex1, enterprise_fee: create(:enterprise_fee, enterprise: ex1.sender)) ExchangeFee.create!(exchange: ex2, enterprise_fee: create(:enterprise_fee, enterprise: ex2.sender)) # Distributors - ex3 = create(:exchange, :order_cycle => oc, :sender => oc.coordinator, :pickup_time => 'time 0', :pickup_instructions => 'instructions 0') - ex4 = create(:exchange, :order_cycle => oc, :sender => oc.coordinator, :pickup_time => 'time 1', :pickup_instructions => 'instructions 1') + ex3 = create(:exchange, :order_cycle => oc, + :sender => oc.coordinator, :receiver => create(:distributor_enterprise), + :pickup_time => 'time 0', :pickup_instructions => 'instructions 0') + ex4 = create(:exchange, :order_cycle => oc, + :sender => oc.coordinator, :receiver => create(:distributor_enterprise), + :pickup_time => 'time 1', :pickup_instructions => 'instructions 1') ExchangeFee.create!(exchange: ex3, enterprise_fee: create(:enterprise_fee, enterprise: ex3.receiver)) ExchangeFee.create!(exchange: ex4, @@ -30,6 +36,11 @@ FactoryGirl.define do exchange.variants << product.master end + + variants = [ex1, ex2].map(&:variants).flatten + [ex3, ex4].each do |exchange| + variants.each { |v| exchange.variants << v } + end end end @@ -84,12 +95,13 @@ FactoryGirl.define do is_distributor true end + sequence(:calculator_amount) factory :enterprise_fee, :class => EnterpriseFee do sequence(:name) { |n| "Enterprise fee #{n}" } + sequence(:fee_type) { |n| EnterpriseFee::FEE_TYPES[n % EnterpriseFee::FEE_TYPES.count] } enterprise { Enterprise.first || FactoryGirl.create(:supplier_enterprise) } - fee_type 'packing' - calculator { FactoryGirl.build(:weight_calculator) } + calculator { FactoryGirl.build(:calculator, preferred_amount: generate(:calculator_amount)) } after(:create) { |ef| ef.calculator.save! } end diff --git a/spec/features/consumer/checkout_spec.rb b/spec/features/consumer/checkout_spec.rb index b14deda33e..58be496f59 100644 --- a/spec/features/consumer/checkout_spec.rb +++ b/spec/features/consumer/checkout_spec.rb @@ -18,6 +18,8 @@ feature %q{ end background do + set_feature_toggle :order_cycles, true + @distributor = create(:distributor_enterprise, :name => 'Edible garden', :address => create(:address, :address1 => '12 Bungee Rd', @@ -37,11 +39,11 @@ feature %q{ :country => Spree::Country.find_by_name('Australia')), :pickup_times => 'Tuesday, 4 PM') - @enterprise_fee_1 = create(:enterprise_fee, :name => 'Shipping Method One', :calculator => Spree::Calculator::FlatRate.new) + @enterprise_fee_1 = create(:enterprise_fee, :name => 'Enterprise Fee One', :calculator => Spree::Calculator::FlatRate.new) @enterprise_fee_1.calculator.set_preference :amount, 1 @enterprise_fee_1.calculator.save! - @enterprise_fee_2 = create(:enterprise_fee, :name => 'Shipping Method Two', :calculator => Spree::Calculator::FlatRate.new) + @enterprise_fee_2 = create(:enterprise_fee, :name => 'Enterprise Fee Two', :calculator => Spree::Calculator::FlatRate.new) @enterprise_fee_2.calculator.set_preference :amount, 2 @enterprise_fee_2.calculator.save! @@ -53,6 +55,7 @@ feature %q{ @product_2.product_distributions.create(:distributor => @distributor, :enterprise_fee => @enterprise_fee_2) @product_2.product_distributions.create(:distributor => @distributor_alternative, :enterprise_fee => @enterprise_fee_2) + # -- Shipping @zone = create(:zone) c = Spree::Country.find_by_name('Australia') Spree::ZoneMember.create(:zoneable => c, :zone => @zone) @@ -85,7 +88,46 @@ feature %q{ page.should have_selector 'span.distribution-total', :text => '$3.00' end - #scenario "viewing delivery fees for order cycle distribution" + scenario "viewing delivery fees for order cycle distribution" do + # Given an order cycle + make_order_cycle + + # And I am logged in + login_to_consumer_section + + # When I add some bananas and zucchini to my cart + click_link 'Bananas' + select @distributor_oc.name, :from => 'distributor_id' + select @order_cycle.name, :from => 'order_cycle_id' + click_button 'Add To Cart' + click_link 'Continue shopping' + + click_link 'Zucchini' + click_button 'Add To Cart' + + # Then I should see a breakdown of my delivery fees: + table = page.find 'tbody#cart_adjustments' + rows = table.all 'tr' + + binding.pry + + rows.map { |row| row.all('td').map { |cell| cell.text.strip } }.should == + [["Bananas - packing fee by supplier Supplier 1", "$3.00", ""], + ["Bananas - transport fee by supplier Supplier 1", "$4.00", ""], + ["Bananas - packing fee by distributor Distributor 1", "$7.00", ""], + ["Bananas - transport fee by distributor Distributor 1", "$8.00", ""], + ["Bananas - admin fee by coordinator My coordinator", "$1.00", ""], + ["Bananas - sales fee by coordinator My coordinator", "$2.00", ""], + ["Zucchini - admin fee by supplier Supplier 2", "$5.00", ""], + ["Zucchini - sales fee by supplier Supplier 2", "$6.00", ""], + ["Zucchini - packing fee by distributor Distributor 1", "$7.00", ""], + ["Zucchini - transport fee by distributor Distributor 1", "$8.00", ""], + ["Zucchini - admin fee by coordinator My coordinator", "$1.00", ""], + ["Zucchini - sales fee by coordinator My coordinator", "$2.00", ""]] + + page.should have_selector 'span.distribution-total', :text => '$54.00' + end + #scenario "viewing delivery fees for mixed product and order cycle distribution" scenario "changing distributor updates delivery fees" do @@ -216,4 +258,62 @@ feature %q{ # page.should have_content('On Tuesday, 4 PM') # page.should have_content('12 Bungee Rd, Carion') end + + + private + + def make_order_cycle + @order_cycle = oc = create(:simple_order_cycle, coordinator: create(:distributor_enterprise, name: 'My coordinator')) + + # Coordinator + coordinator_fee1 = create(:enterprise_fee, enterprise: oc.coordinator, fee_type: 'admin') + coordinator_fee2 = create(:enterprise_fee, enterprise: oc.coordinator, fee_type: 'sales') + oc.coordinator_fees << coordinator_fee1 + oc.coordinator_fees << coordinator_fee2 + + # Suppliers + supplier1 = create(:supplier_enterprise, name: 'Supplier 1') + supplier2 = create(:supplier_enterprise, name: 'Supplier 2') + supplier_fee1 = create(:enterprise_fee, enterprise: supplier1, fee_type: 'packing') + supplier_fee2 = create(:enterprise_fee, enterprise: supplier1, fee_type: 'transport') + supplier_fee3 = create(:enterprise_fee, enterprise: supplier2, fee_type: 'admin') + supplier_fee4 = create(:enterprise_fee, enterprise: supplier2, fee_type: 'sales') + ex1 = create(:exchange, order_cycle: oc, sender: supplier1, receiver: oc.coordinator) + ex2 = create(:exchange, order_cycle: oc, sender: supplier2, receiver: oc.coordinator) + ExchangeFee.create!(exchange: ex1, enterprise_fee: supplier_fee1) + ExchangeFee.create!(exchange: ex1, enterprise_fee: supplier_fee2) + ExchangeFee.create!(exchange: ex2, enterprise_fee: supplier_fee3) + ExchangeFee.create!(exchange: ex2, enterprise_fee: supplier_fee4) + + # Distributors + distributor1 = create(:distributor_enterprise, name: 'Distributor 1') + distributor2 = create(:distributor_enterprise, name: 'Distributor 2') + distributor_fee1 = create(:enterprise_fee, enterprise: distributor1, fee_type: 'packing') + distributor_fee2 = create(:enterprise_fee, enterprise: distributor1, fee_type: 'transport') + distributor_fee3 = create(:enterprise_fee, enterprise: distributor2, fee_type: 'admin') + distributor_fee4 = create(:enterprise_fee, enterprise: distributor2, fee_type: 'sales') + ex3 = create(:exchange, order_cycle: oc, + sender: oc.coordinator, receiver: distributor1, + pickup_time: 'time 0', pickup_instructions: 'instructions 0') + ex4 = create(:exchange, order_cycle: oc, + sender: oc.coordinator, receiver: distributor2, + pickup_time: 'time 1', pickup_instructions: 'instructions 1') + ExchangeFee.create!(exchange: ex3, enterprise_fee: distributor_fee1) + ExchangeFee.create!(exchange: ex3, enterprise_fee: distributor_fee2) + ExchangeFee.create!(exchange: ex4, enterprise_fee: distributor_fee3) + ExchangeFee.create!(exchange: ex4, enterprise_fee: distributor_fee4) + + # Products + @distributor_oc = distributor1 + + @product_3 = create(:simple_product, name: 'Bananas', supplier: supplier1) + ex1.variants << @product_3.master + ex3.variants << @product_3.master + ex4.variants << @product_3.master + + @product_4 = create(:simple_product, name: 'Zucchini', supplier: supplier2) + ex2.variants << @product_4.master + ex3.variants << @product_4.master + ex4.variants << @product_4.master + end end From 24cd5209d68ad5c304b63bf185a9f6b7c8b34107 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 19 Aug 2013 10:41:01 +1000 Subject: [PATCH 12/30] Do not error when attempting to mix product and order cycle distribution --- ..._to_cart_order_cycle_unavailable.html.haml | 2 +- spec/features/consumer/checkout_spec.rb | 26 ++++++++++++++++--- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/app/views/spree/products/_add_to_cart_order_cycle_unavailable.html.haml b/app/views/spree/products/_add_to_cart_order_cycle_unavailable.html.haml index ba3a99dbbb..d061f0595c 100644 --- a/app/views/spree/products/_add_to_cart_order_cycle_unavailable.html.haml +++ b/app/views/spree/products/_add_to_cart_order_cycle_unavailable.html.haml @@ -1,4 +1,4 @@ .error-distributor Please complete your order from - = link_to current_order_cycle.name, root_path + = link_to (current_order_cycle.andand.name || 'your current order cycle'), root_path before shopping in a different order cycle. diff --git a/spec/features/consumer/checkout_spec.rb b/spec/features/consumer/checkout_spec.rb index 58be496f59..8717e6062a 100644 --- a/spec/features/consumer/checkout_spec.rb +++ b/spec/features/consumer/checkout_spec.rb @@ -109,8 +109,6 @@ feature %q{ table = page.find 'tbody#cart_adjustments' rows = table.all 'tr' - binding.pry - rows.map { |row| row.all('td').map { |cell| cell.text.strip } }.should == [["Bananas - packing fee by supplier Supplier 1", "$3.00", ""], ["Bananas - transport fee by supplier Supplier 1", "$4.00", ""], @@ -128,7 +126,29 @@ feature %q{ page.should have_selector 'span.distribution-total', :text => '$54.00' end - #scenario "viewing delivery fees for mixed product and order cycle distribution" + scenario "attempting to purchase products that mix product and order cycle distribution" do + # Given some products, one with product distribution only, (@product1) + # one with order cycle distribution only, (@product_oc) + supplier = create(:supplier_enterprise) + product_oc = create(:simple_product, name: 'Feijoas') + @order_cycle = create(:simple_order_cycle, suppliers: [supplier], distributors: [@distributor], variants: [product_oc.master]) + @order_cycle.coordinator_fees << create(:enterprise_fee, enterprise: @order_cycle.coordinator) + + # And I am logged in + login_to_consumer_section + + # When I add the first to my cart + click_link 'Fuji apples' + select @distributor.name, :from => 'distributor_id' + click_button 'Add To Cart' + click_link 'Continue shopping' + + # And I attempt to add another + click_link 'Feijoas' + + # Then I should see an error about changing order cycle + page.should have_content 'Please complete your order from your current order cycle before shopping in a different order cycle.' + end scenario "changing distributor updates delivery fees" do # Given two distributors and enterprise fees From 6c24c0ef686a551a3cd44322709805256855c880 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 19 Aug 2013 11:10:00 +1000 Subject: [PATCH 13/30] Make specs more resilient and less context-sensitive --- spec/factories.rb | 4 +++- spec/features/admin/enterprise_fees_spec.rb | 7 ++++--- spec/features/admin/order_cycles_spec.rb | 3 ++- spec/features/consumer/checkout_spec.rb | 20 ++++++++++---------- 4 files changed, 19 insertions(+), 15 deletions(-) diff --git a/spec/factories.rb b/spec/factories.rb index 1a0425f337..6038f62cea 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -97,11 +97,13 @@ FactoryGirl.define do sequence(:calculator_amount) factory :enterprise_fee, :class => EnterpriseFee do + ignore { amount nil } + sequence(:name) { |n| "Enterprise fee #{n}" } sequence(:fee_type) { |n| EnterpriseFee::FEE_TYPES[n % EnterpriseFee::FEE_TYPES.count] } enterprise { Enterprise.first || FactoryGirl.create(:supplier_enterprise) } - calculator { FactoryGirl.build(:calculator, preferred_amount: generate(:calculator_amount)) } + calculator { FactoryGirl.build(:calculator, preferred_amount: amount || generate(:calculator_amount)) } after(:create) { |ef| ef.calculator.save! } end diff --git a/spec/features/admin/enterprise_fees_spec.rb b/spec/features/admin/enterprise_fees_spec.rb index e2af5bb65e..71e262b70d 100644 --- a/spec/features/admin/enterprise_fees_spec.rb +++ b/spec/features/admin/enterprise_fees_spec.rb @@ -17,7 +17,8 @@ feature %q{ end scenario "listing enterprise fees" do - fee = create(:enterprise_fee, name: '$0.50 / kg') + fee = create(:enterprise_fee, name: '$0.50 / kg', fee_type: 'packing') + amount = fee.calculator.preferred_amount login_to_admin_section click_link 'Configuration' @@ -26,8 +27,8 @@ feature %q{ page.should have_selector "#enterprise_fee_set_collection_attributes_0_enterprise_id" page.should have_selector "option[selected]", text: 'Packing' page.should have_selector "input[value='$0.50 / kg']" - page.should have_selector "option[selected]", text: 'Weight (per kg)' - page.should have_selector "input[value='0.5']" + page.should have_selector "option[selected]", text: 'Flat Rate (per order)' + page.should have_selector "input[value='#{amount}']" end scenario "creating an enterprise fee" do diff --git a/spec/features/admin/order_cycles_spec.rb b/spec/features/admin/order_cycles_spec.rb index c1bada0060..9697504901 100644 --- a/spec/features/admin/order_cycles_spec.rb +++ b/spec/features/admin/order_cycles_spec.rb @@ -206,6 +206,7 @@ feature %q{ scenario "updating an order cycle" do # Given an order cycle with all the settings oc = create(:order_cycle) + initial_variants = oc.variants # And a coordinating, supplying and distributing enterprise with some products with variants coordinator = create(:distributor_enterprise, name: 'My coordinator') @@ -311,7 +312,7 @@ feature %q{ OrderCycle.last.exchanges.outgoing.last.enterprise_fee_ids.should == [distributor_fee2.id] # And it should have some variants selected - OrderCycle.last.variants.map { |v| v.id }.sort.should == [1, v1.id, v2.id].sort + OrderCycle.last.variants.map(&:id).sort.should == (initial_variants.map(&:id) + [v1.id, v2.id]).sort # And the collection details should have been updated OrderCycle.last.exchanges.where(pickup_time: 'New time 0', pickup_instructions: 'New instructions 0').should be_present diff --git a/spec/features/consumer/checkout_spec.rb b/spec/features/consumer/checkout_spec.rb index 8717e6062a..2dbc285849 100644 --- a/spec/features/consumer/checkout_spec.rb +++ b/spec/features/consumer/checkout_spec.rb @@ -286,18 +286,18 @@ feature %q{ @order_cycle = oc = create(:simple_order_cycle, coordinator: create(:distributor_enterprise, name: 'My coordinator')) # Coordinator - coordinator_fee1 = create(:enterprise_fee, enterprise: oc.coordinator, fee_type: 'admin') - coordinator_fee2 = create(:enterprise_fee, enterprise: oc.coordinator, fee_type: 'sales') + coordinator_fee1 = create(:enterprise_fee, enterprise: oc.coordinator, fee_type: 'admin', amount: 1) + coordinator_fee2 = create(:enterprise_fee, enterprise: oc.coordinator, fee_type: 'sales', amount: 2) oc.coordinator_fees << coordinator_fee1 oc.coordinator_fees << coordinator_fee2 # Suppliers supplier1 = create(:supplier_enterprise, name: 'Supplier 1') supplier2 = create(:supplier_enterprise, name: 'Supplier 2') - supplier_fee1 = create(:enterprise_fee, enterprise: supplier1, fee_type: 'packing') - supplier_fee2 = create(:enterprise_fee, enterprise: supplier1, fee_type: 'transport') - supplier_fee3 = create(:enterprise_fee, enterprise: supplier2, fee_type: 'admin') - supplier_fee4 = create(:enterprise_fee, enterprise: supplier2, fee_type: 'sales') + supplier_fee1 = create(:enterprise_fee, enterprise: supplier1, fee_type: 'packing', amount: 3) + supplier_fee2 = create(:enterprise_fee, enterprise: supplier1, fee_type: 'transport', amount: 4) + supplier_fee3 = create(:enterprise_fee, enterprise: supplier2, fee_type: 'admin', amount: 5) + supplier_fee4 = create(:enterprise_fee, enterprise: supplier2, fee_type: 'sales', amount: 6) ex1 = create(:exchange, order_cycle: oc, sender: supplier1, receiver: oc.coordinator) ex2 = create(:exchange, order_cycle: oc, sender: supplier2, receiver: oc.coordinator) ExchangeFee.create!(exchange: ex1, enterprise_fee: supplier_fee1) @@ -308,10 +308,10 @@ feature %q{ # Distributors distributor1 = create(:distributor_enterprise, name: 'Distributor 1') distributor2 = create(:distributor_enterprise, name: 'Distributor 2') - distributor_fee1 = create(:enterprise_fee, enterprise: distributor1, fee_type: 'packing') - distributor_fee2 = create(:enterprise_fee, enterprise: distributor1, fee_type: 'transport') - distributor_fee3 = create(:enterprise_fee, enterprise: distributor2, fee_type: 'admin') - distributor_fee4 = create(:enterprise_fee, enterprise: distributor2, fee_type: 'sales') + distributor_fee1 = create(:enterprise_fee, enterprise: distributor1, fee_type: 'packing', amount: 7) + distributor_fee2 = create(:enterprise_fee, enterprise: distributor1, fee_type: 'transport', amount: 8) + distributor_fee3 = create(:enterprise_fee, enterprise: distributor2, fee_type: 'admin', amount: 9) + distributor_fee4 = create(:enterprise_fee, enterprise: distributor2, fee_type: 'sales', amount: 10) ex3 = create(:exchange, order_cycle: oc, sender: oc.coordinator, receiver: distributor1, pickup_time: 'time 0', pickup_instructions: 'instructions 0') From ba4d3d5d7dc726e0136cb7eb518bc347a75f818e Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 19 Aug 2013 12:19:03 +1000 Subject: [PATCH 14/30] Removing a product from cart removes its fees --- app/models/enterprise_fee.rb | 5 +++ app/models/order_cycle.rb | 9 ++--- app/models/product_distribution.rb | 5 --- app/models/spree/order_decorator.rb | 6 ++-- spec/features/consumer/checkout_spec.rb | 34 +++++++++++++++++-- spec/models/enterprise_fee_spec.rb | 38 +++++++++++++++++++++ spec/models/order_cycle_spec.rb | 17 ---------- spec/models/order_spec.rb | 18 +++++++--- spec/models/product_distribution_spec.rb | 42 ------------------------ 9 files changed, 94 insertions(+), 80 deletions(-) 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) From c1f9d9789da90059f468ede0aed99fb7d3de0bb7 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 19 Aug 2013 12:29:26 +1000 Subject: [PATCH 15/30] Extract table test to private method --- spec/features/consumer/checkout_spec.rb | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/spec/features/consumer/checkout_spec.rb b/spec/features/consumer/checkout_spec.rb index f0a8ec903c..9cd7ee1925 100644 --- a/spec/features/consumer/checkout_spec.rb +++ b/spec/features/consumer/checkout_spec.rb @@ -81,10 +81,7 @@ feature %q{ click_button 'Add To Cart' # Then I should see a breakdown of my delivery fees: - table = page.find 'tbody#cart_adjustments' - rows = table.all 'tr' - - rows.map { |row| row.all('td').map { |cell| cell.text.strip } }.should == + checkout_fees_table.should == [['Product distribution by Edible garden for Fuji apples', '$1.00', ''], ['Product distribution by Edible garden for Garlic', '$2.00', '']] @@ -109,10 +106,7 @@ feature %q{ click_button 'Add To Cart' # Then I should see a breakdown of my delivery fees: - table = page.find 'tbody#cart_adjustments' - rows = table.all 'tr' - - rows.map { |row| row.all('td').map { |cell| cell.text.strip } }.should == + checkout_fees_table.should == [["Bananas - packing fee by supplier Supplier 1", "$3.00", ""], ["Bananas - transport fee by supplier Supplier 1", "$4.00", ""], ["Bananas - packing fee by distributor Distributor 1", "$7.00", ""], @@ -171,10 +165,7 @@ feature %q{ 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 == + checkout_fees_table.should == [['Product distribution by Edible garden for Garlic', '$2.00', '']] page.should have_selector 'span.distribution-total', :text => '$2.00' @@ -366,4 +357,10 @@ feature %q{ ex3.variants << @product_4.master ex4.variants << @product_4.master end + + def checkout_fees_table + table = page.find 'tbody#cart_adjustments' + rows = table.all 'tr' + rows.map { |row| row.all('td').map { |cell| cell.text.strip } } + end end From 4be2fe5fe9fcb722761769b800b2a7990b42d5f2 Mon Sep 17 00:00:00 2001 From: David Cook Date: Mon, 19 Aug 2013 11:29:54 +1000 Subject: [PATCH 16/30] Only show payment methods that user has access to --- .../admin/payment_methods_controller_decorator.rb | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 app/controllers/spree/admin/payment_methods_controller_decorator.rb diff --git a/app/controllers/spree/admin/payment_methods_controller_decorator.rb b/app/controllers/spree/admin/payment_methods_controller_decorator.rb new file mode 100644 index 0000000000..d1d367fc47 --- /dev/null +++ b/app/controllers/spree/admin/payment_methods_controller_decorator.rb @@ -0,0 +1,14 @@ +Spree::Admin::PaymentMethodsController.class_eval do + # Only show payment methods that user has access to. + # ! Redundant code copied from Spree::Admin::ResourceController with two added lines + def collection + return parent.send(controller_name) if parent_data.present? + if model_class.respond_to?(:accessible_by) && !current_ability.has_block?(params[:action], model_class) + model_class.accessible_by(current_ability, action). + managed_by(spree_current_user) # this line added + else + model_class.scoped. + managed_by(spree_current_user) # this line added + end + end +end \ No newline at end of file From 77329b5532bb776a688dd6645cee50072a2a7371 Mon Sep 17 00:00:00 2001 From: David Cook Date: Mon, 19 Aug 2013 11:31:39 +1000 Subject: [PATCH 17/30] Allow user to delete their enterprises' own payment methods --- app/models/spree/ability_decorator.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb index 34bd9679a4..eae10efb66 100644 --- a/app/models/spree/ability_decorator.rb +++ b/app/models/spree/ability_decorator.rb @@ -30,7 +30,7 @@ class AbilityDecorator #Enterprise User can only access payment methods for their distributors can [:index, :create], Spree::PaymentMethod - can [:admin, :read, :update, :fire, :resend ], Spree::PaymentMethod do |payment_method| + can [:admin, :read, :update, :fire, :resend, :destroy ], Spree::PaymentMethod do |payment_method| user.enterprises.include? payment_method.distributor end From 6f73f417002697a5052c9199668fb67b354cd8b9 Mon Sep 17 00:00:00 2001 From: David Cook Date: Mon, 19 Aug 2013 12:04:45 +1000 Subject: [PATCH 18/30] Create Payment Methods for each Enterprise instead of generic Payment Method --- lib/tasks/dev.rake | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/tasks/dev.rake b/lib/tasks/dev.rake index 6fce4ebf81..47bd8ca4b3 100644 --- a/lib/tasks/dev.rake +++ b/lib/tasks/dev.rake @@ -16,7 +16,6 @@ namespace :openfoodweb do country = Spree::Country.find_by_name('Australia') Spree::ZoneMember.create(:zone => zone, :zoneable => country) FactoryGirl.create(:shipping_method, :zone => zone) - FactoryGirl.create(:payment_method, :environment => 'development') end @@ -96,6 +95,13 @@ namespace :openfoodweb do end end + # -- Enterprise Payment Methods + unless Spree::PaymentMethod.count > 1 + Enterprise.is_distributor.each do |distributor| + FactoryGirl.create(:payment_method, distributor: distributor, name: "Cheque (#{distributor.name})", :environment => 'development') + end + end + # -- Products unless Spree::Product.count > 0 puts "[#{task_name}] Seeding products" From 08babeed65ab97d8df89ea70f5f1177169f90f08 Mon Sep 17 00:00:00 2001 From: David Cook Date: Mon, 19 Aug 2013 12:05:25 +1000 Subject: [PATCH 19/30] Payment Method must have one Distributor --- app/models/spree/payment_method_decorator.rb | 2 ++ ...dd_distributor_to_admin_payment_method_edit.html.haml.deface | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/models/spree/payment_method_decorator.rb b/app/models/spree/payment_method_decorator.rb index f8759ac06b..095b6b5dfc 100644 --- a/app/models/spree/payment_method_decorator.rb +++ b/app/models/spree/payment_method_decorator.rb @@ -1,5 +1,7 @@ Spree::PaymentMethod.class_eval do belongs_to :distributor, :class_name => 'Enterprise' + + validates_presence_of :distributor_id attr_accessible :distributor_id diff --git a/app/overrides/spree/admin/payment_methods/_form/add_distributor_to_admin_payment_method_edit.html.haml.deface b/app/overrides/spree/admin/payment_methods/_form/add_distributor_to_admin_payment_method_edit.html.haml.deface index 8ac8e7aad9..d15ecec373 100644 --- a/app/overrides/spree/admin/payment_methods/_form/add_distributor_to_admin_payment_method_edit.html.haml.deface +++ b/app/overrides/spree/admin/payment_methods/_form/add_distributor_to_admin_payment_method_edit.html.haml.deface @@ -5,5 +5,5 @@ = f.label :distributor %br - = collection_select(:payment_method, :distributor_id, Enterprise.is_distributor.managed_by(spree_current_user), :id, :name, {:include_blank => true}, {:class => "select2 fullwidth"}) + = collection_select(:payment_method, :distributor_id, Enterprise.is_distributor.managed_by(spree_current_user), :id, :name, {:include_blank => false}, {:class => "select2 fullwidth"}) = f.error_message_on :distributor From 2d305b59d950fe1d1bf7c5757d6f9ad3f94d24c5 Mon Sep 17 00:00:00 2001 From: David Cook Date: Mon, 19 Aug 2013 12:43:45 +1000 Subject: [PATCH 20/30] Only show payment methods for the distributor of the order --- .../admin/payments_controller_decorator.rb | 24 ++++++++++++------- app/models/spree/payment_method_decorator.rb | 6 ++++- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/app/controllers/spree/admin/payments_controller_decorator.rb b/app/controllers/spree/admin/payments_controller_decorator.rb index 4a58dbfe14..35237d3e76 100644 --- a/app/controllers/spree/admin/payments_controller_decorator.rb +++ b/app/controllers/spree/admin/payments_controller_decorator.rb @@ -1,14 +1,20 @@ -# When a user fires an event, take them back to where they came from -# Responder: http://guides.spreecommerce.com/developer/logic.html#overriding-controller-action-responses - -# For some strange reason, adding PaymentsController.class_eval will cause gems/spree/app/controllers/spree/admin/payments_controller.rb:37 to error: -# payments_url not defined. -# This could be fixed by replacing line 37 with: -# respond_with(@payment, location: admin_order_payments_url) { |format| format.html { redirect_to admin_order_payments_path(@order) } } - - Spree::Admin::PaymentsController.class_eval do + # When a user fires an event, take them back to where they came from + # Responder: http://guides.spreecommerce.com/developer/logic.html#overriding-controller-action-responses + + # For some strange reason, adding PaymentsController.class_eval will cause gems/spree/app/controllers/spree/admin/payments_controller.rb:37 to error: + # payments_url not defined. + # This could be fixed by replacing line 37 with: + # respond_with(@payment, location: admin_order_payments_url) { |format| format.html { redirect_to admin_order_payments_path(@order) } } respond_override :fire => { :html => { :success => lambda { redirect_to request.referer # Keeps any filter and sort prefs } } } + + append_before_filter :filter_payment_methods + + # Only show payments for the order's distributor + def filter_payment_methods + @payment_methods = @payment_methods.select{ |pm| pm.has_distributor? @order.distributor} + end + end diff --git a/app/models/spree/payment_method_decorator.rb b/app/models/spree/payment_method_decorator.rb index 095b6b5dfc..1f19d9498c 100644 --- a/app/models/spree/payment_method_decorator.rb +++ b/app/models/spree/payment_method_decorator.rb @@ -1,6 +1,6 @@ Spree::PaymentMethod.class_eval do belongs_to :distributor, :class_name => 'Enterprise' - + validates_presence_of :distributor_id attr_accessible :distributor_id @@ -17,6 +17,10 @@ Spree::PaymentMethod.class_eval do } end +def has_distributor?(distributor) + self.distributor == distributor +end + # Ensure that all derived classes also allow distributor_id Spree::Gateway.providers.each do |p| p.attr_accessible :distributor_id From 3ffe732b3057bfa28e847eef2534df88ac7d3842 Mon Sep 17 00:00:00 2001 From: David Cook Date: Mon, 19 Aug 2013 12:51:49 +1000 Subject: [PATCH 21/30] Ensure valid payment method is selected --- .../spree/admin/payments_controller_decorator.rb | 1 + app/models/spree/payment_method_decorator.rb | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/app/controllers/spree/admin/payments_controller_decorator.rb b/app/controllers/spree/admin/payments_controller_decorator.rb index 35237d3e76..23db5eca09 100644 --- a/app/controllers/spree/admin/payments_controller_decorator.rb +++ b/app/controllers/spree/admin/payments_controller_decorator.rb @@ -15,6 +15,7 @@ Spree::Admin::PaymentsController.class_eval do # Only show payments for the order's distributor def filter_payment_methods @payment_methods = @payment_methods.select{ |pm| pm.has_distributor? @order.distributor} + @payment_method ||= @payment_methods.first end end diff --git a/app/models/spree/payment_method_decorator.rb b/app/models/spree/payment_method_decorator.rb index 1f19d9498c..0f52a4e291 100644 --- a/app/models/spree/payment_method_decorator.rb +++ b/app/models/spree/payment_method_decorator.rb @@ -15,10 +15,10 @@ Spree::PaymentMethod.class_eval do where('distributor_id IN (?)', user.enterprises.map {|enterprise| enterprise.id }) end } -end -def has_distributor?(distributor) - self.distributor == distributor + def has_distributor?(distributor) + self.distributor == distributor + end end # Ensure that all derived classes also allow distributor_id From a9b70e67df9ebf27f5061c89992cf7d300f34689 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 19 Aug 2013 14:26:12 +1000 Subject: [PATCH 22/30] Change to using flat rate per item calculators - per-order is confusing when used against line items --- spec/factories.rb | 2 +- spec/features/consumer/checkout_spec.rb | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/factories.rb b/spec/factories.rb index 6038f62cea..2ce408807f 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -103,7 +103,7 @@ FactoryGirl.define do sequence(:fee_type) { |n| EnterpriseFee::FEE_TYPES[n % EnterpriseFee::FEE_TYPES.count] } enterprise { Enterprise.first || FactoryGirl.create(:supplier_enterprise) } - calculator { FactoryGirl.build(:calculator, preferred_amount: amount || generate(:calculator_amount)) } + calculator { Spree::Calculator::PerItem.new(preferred_amount: amount || generate(:calculator_amount)) } after(:create) { |ef| ef.calculator.save! } end diff --git a/spec/features/consumer/checkout_spec.rb b/spec/features/consumer/checkout_spec.rb index 9cd7ee1925..e4bb843ad3 100644 --- a/spec/features/consumer/checkout_spec.rb +++ b/spec/features/consumer/checkout_spec.rb @@ -39,11 +39,11 @@ feature %q{ :country => Spree::Country.find_by_name('Australia')), :pickup_times => 'Tuesday, 4 PM') - @enterprise_fee_1 = create(:enterprise_fee, :name => 'Enterprise Fee One', :calculator => Spree::Calculator::FlatRate.new) + @enterprise_fee_1 = create(:enterprise_fee, :name => 'Enterprise Fee One', :calculator => Spree::Calculator::PerItem.new) @enterprise_fee_1.calculator.set_preference :amount, 1 @enterprise_fee_1.calculator.save! - @enterprise_fee_2 = create(:enterprise_fee, :name => 'Enterprise Fee Two', :calculator => Spree::Calculator::FlatRate.new) + @enterprise_fee_2 = create(:enterprise_fee, :name => 'Enterprise Fee Two', :calculator => Spree::Calculator::PerItem.new) @enterprise_fee_2.calculator.set_preference :amount, 2 @enterprise_fee_2.calculator.save! @@ -175,9 +175,9 @@ feature %q{ # Given two distributors and enterprise fees d1 = create(:distributor_enterprise) d2 = create(:distributor_enterprise) - ef1 = create(:enterprise_fee, calculator: Spree::Calculator::FlatRate.new) + ef1 = create(:enterprise_fee, calculator: Spree::Calculator::PerItem.new) ef1.calculator.set_preference :amount, 1.23; ef1.calculator.save! - ef2 = create(:enterprise_fee, calculator: Spree::Calculator::FlatRate.new) + ef2 = create(:enterprise_fee, calculator: Spree::Calculator::PerItem.new) ef2.calculator.set_preference :amount, 2.34; ef2.calculator.save! # And two products both available from both distributors From 562365311e5d6b29bfe513db2c0ce84807daaccc Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 19 Aug 2013 14:38:30 +1000 Subject: [PATCH 23/30] Lock enterprise fee adjustments on creation to avoid them being recalculated against order by update hooks --- app/models/enterprise_fee.rb | 12 +++++++++ app/models/order_cycle.rb | 2 +- app/models/product_distribution.rb | 3 +-- spec/features/consumer/checkout_spec.rb | 34 ++++++++++++++++++++++++ spec/models/product_distribution_spec.rb | 6 ++--- 5 files changed, 51 insertions(+), 6 deletions(-) diff --git a/app/models/enterprise_fee.rb b/app/models/enterprise_fee.rb index ad13a36c7a..05f3828ecd 100644 --- a/app/models/enterprise_fee.rb +++ b/app/models/enterprise_fee.rb @@ -22,4 +22,16 @@ class EnterpriseFee < ActiveRecord::Base order.adjustments.where(originator_type: 'EnterpriseFee', source_type: 'Spree::LineItem').destroy_all end + # Create an adjustment that starts as locked. Preferable to making an adjustment and locking it since + # the unlocked adjustment tends to get hit by callbacks before we have a chance to lock it. + def create_locked_adjustment(label, target, calculable, mandatory=false) + amount = compute_amount(calculable) + return if amount == 0 && !mandatory + target.adjustments.create({ :amount => amount, + :source => calculable, + :originator => self, + :label => label, + :mandatory => mandatory, + :locked => true}, :without_protection => true) + end end diff --git a/app/models/order_cycle.rb b/app/models/order_cycle.rb index f8cb0a35a1..88bcdefd7e 100644 --- a/app/models/order_cycle.rb +++ b/app/models/order_cycle.rb @@ -92,7 +92,7 @@ class OrderCycle < ActiveRecord::Base end def create_adjustment_for_fee(line_item, enterprise_fee, label, role) - a = enterprise_fee.create_adjustment(label, line_item.order, line_item, true) + a = enterprise_fee.create_locked_adjustment(label, line_item.order, line_item, true) AdjustmentMetadata.create! adjustment: a, enterprise: enterprise_fee.enterprise, fee_name: enterprise_fee.name, fee_type: enterprise_fee.fee_type, enterprise_role: role end diff --git a/app/models/product_distribution.rb b/app/models/product_distribution.rb index afc39ca2bf..22a96dc5f0 100644 --- a/app/models/product_distribution.rb +++ b/app/models/product_distribution.rb @@ -17,11 +17,10 @@ class ProductDistribution < ActiveRecord::Base end def create_adjustment_for(line_item) - a = enterprise_fee.create_adjustment(adjustment_label_for(line_item), line_item.order, line_item, true) + a = enterprise_fee.create_locked_adjustment(adjustment_label_for(line_item), line_item.order, line_item, true) AdjustmentMetadata.create! adjustment: a, enterprise: enterprise_fee.enterprise, fee_name: enterprise_fee.name, fee_type: enterprise_fee.fee_type, enterprise_role: 'distributor' end - def adjustment_label_for(line_item) "Product distribution by #{distributor.name} for #{line_item.product.name}" end diff --git a/spec/features/consumer/checkout_spec.rb b/spec/features/consumer/checkout_spec.rb index e4bb843ad3..b12ba2898c 100644 --- a/spec/features/consumer/checkout_spec.rb +++ b/spec/features/consumer/checkout_spec.rb @@ -51,6 +51,10 @@ feature %q{ @product_1.product_distributions.create(:distributor => @distributor, :enterprise_fee => @enterprise_fee_1) @product_1.product_distributions.create(:distributor => @distributor_alternative, :enterprise_fee => @enterprise_fee_1) + @product_1a = create(:product, :name => 'Sundowner apples') + @product_1a.product_distributions.create(:distributor => @distributor, :enterprise_fee => @enterprise_fee_1) + @product_1a.product_distributions.create(:distributor => @distributor_alternative, :enterprise_fee => @enterprise_fee_1) + @product_2 = create(:product, :name => 'Garlic') @product_2.product_distributions.create(:distributor => @distributor, :enterprise_fee => @enterprise_fee_2) @product_2.product_distributions.create(:distributor => @distributor_alternative, :enterprise_fee => @enterprise_fee_2) @@ -171,6 +175,36 @@ feature %q{ page.should have_selector 'span.distribution-total', :text => '$2.00' end + scenario "adding products with differing quantities produces correct fees" do + # Given I am logged in + login_to_consumer_section + + # When I add two products to my cart that share the same enterprise fee + click_link 'Fuji apples' + select @distributor.name, :from => 'distributor_id' + click_button 'Add To Cart' + click_link 'Continue shopping' + + click_link 'Sundowner apples' + click_button 'Add To Cart' + + # Then I should have some delivery fees + checkout_fees_table.should == + [['Product distribution by Edible garden for Fuji apples', '$1.00', ''], + ['Product distribution by Edible garden for Sundowner apples', '$1.00', '']] + page.should have_selector 'span.distribution-total', :text => '$2.00' + + # And I update the quantity of one of them + fill_in 'order_line_items_attributes_0_quantity', with: 2 + click_button 'Update' + + # Then I should see updated delivery fees + checkout_fees_table.should == + [['Product distribution by Edible garden for Fuji apples', '$2.00', ''], + ['Product distribution by Edible garden for Sundowner apples', '$1.00', '']] + page.should have_selector 'span.distribution-total', :text => '$3.00' + end + scenario "changing distributor updates delivery fees" do # Given two distributors and enterprise fees d1 = create(:distributor_enterprise) diff --git a/spec/models/product_distribution_spec.rb b/spec/models/product_distribution_spec.rb index bec4de0350..a2fbf23873 100644 --- a/spec/models/product_distribution_spec.rb +++ b/spec/models/product_distribution_spec.rb @@ -75,7 +75,7 @@ describe ProductDistribution do 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) + adjustment = pd.enterprise_fee.create_locked_adjustment('foo', line_item.order, line_item, true) pd.send(:adjustment_for, line_item).should == adjustment end @@ -83,8 +83,8 @@ describe ProductDistribution do 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) + pd.enterprise_fee.create_locked_adjustment('one', line_item.order, line_item, true) + pd.enterprise_fee.create_locked_adjustment('two', line_item.order, line_item, true) expect do pd.send(:adjustment_for, line_item) From 5a3ad8e68bb4c8ba2189d4b72d05f25e8545a054 Mon Sep 17 00:00:00 2001 From: David Cook Date: Mon, 19 Aug 2013 16:17:38 +1000 Subject: [PATCH 24/30] Remove test for payment method with no distributor --- spec/features/consumer/checkout_spec.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/spec/features/consumer/checkout_spec.rb b/spec/features/consumer/checkout_spec.rb index b12ba2898c..d85ae84ef4 100644 --- a/spec/features/consumer/checkout_spec.rb +++ b/spec/features/consumer/checkout_spec.rb @@ -65,7 +65,6 @@ feature %q{ Spree::ZoneMember.create(:zoneable => c, :zone => @zone) create(:shipping_method, zone: @zone) - @payment_method_all = create(:payment_method, :name => 'Cheque payment method', :description => 'Cheque payment method') #valid for any distributor @payment_method_distributor = create(:payment_method, :name => 'Edible Garden payment method', :distributor => @distributor) @payment_method_alternative = create(:payment_method, :name => 'Alternative Distributor payment method', :distributor => @distributor_alternative) end @@ -319,7 +318,6 @@ feature %q{ # -- Checkout: Payment # Given the distributor I have selected for my order, I should only see payment methods valid for that distributor - page.should have_selector 'label', :text => @payment_method_all.name page.should have_selector 'label', :text => @payment_method_distributor.name page.should_not have_selector 'label', :text => @payment_method_alternative.name click_checkout_continue_button From 5f7cbe3882689c08b89970be19b93245628c633e Mon Sep 17 00:00:00 2001 From: David Cook Date: Mon, 19 Aug 2013 16:18:50 +1000 Subject: [PATCH 25/30] Ensure distributor is set on payment method factories --- spec/factories.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/spec/factories.rb b/spec/factories.rb index 2ce408807f..7385352ac5 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -158,6 +158,18 @@ FactoryGirl.modify do state { Spree::State.find_by_name 'Victoria' } country { Spree::Country.find_by_name 'Australia' || Spree::Country.first } end + + factory :payment do + ignore do + distributor { order.distributor || Enterprise.is_distributor.first || FactoryGirl.create(:distributor_enterprise) } + end + payment_method { FactoryGirl.create(:payment_method, distributor: distributor) } + end + + factory :payment_method do + distributor { Enterprise.is_distributor.first || FactoryGirl.create(:distributor_enterprise) } #Always need a distributor + end + end From 010c58fffd4e404555168f1a84ad7dc200bc7eaa Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 19 Aug 2013 16:46:51 +1000 Subject: [PATCH 26/30] Ignore libpeerconnection.log from phantomjs --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 78e0a36aef..12c35da681 100644 --- a/.gitignore +++ b/.gitignore @@ -35,3 +35,4 @@ config/heroku_env.rb config/initializers/feature_toggle.rb NERD_tree* coverage +libpeerconnection.log From 693fa9f37bdafceb6bed003d288622c92041cfed Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 19 Aug 2013 16:47:17 +1000 Subject: [PATCH 27/30] Enable enterprises_distributor_info_rich_text by default --- lib/open_food_web/feature_toggle.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/open_food_web/feature_toggle.rb b/lib/open_food_web/feature_toggle.rb index ecb5fd1e1d..00834da4e0 100644 --- a/lib/open_food_web/feature_toggle.rb +++ b/lib/open_food_web/feature_toggle.rb @@ -11,7 +11,7 @@ module OpenFoodWeb local_organics: false, order_cycles: false, multi_cart: false, - enterprises_distributor_info_rich_text: false} + enterprises_distributor_info_rich_text: true} end end end From 0458f7a6bbfcfdc0d7240422eaae2f1c914e961d Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 19 Aug 2013 17:02:02 +1000 Subject: [PATCH 28/30] Summarise distribution fees in checkout after cart page --- app/helpers/checkout_helper.rb | 13 ++++++++ app/views/spree/checkout/_summary.html.erb | 36 ++++++++++++++++++++++ spec/features/consumer/checkout_spec.rb | 7 +++++ 3 files changed, 56 insertions(+) create mode 100644 app/helpers/checkout_helper.rb create mode 100644 app/views/spree/checkout/_summary.html.erb diff --git a/app/helpers/checkout_helper.rb b/app/helpers/checkout_helper.rb new file mode 100644 index 0000000000..b21855bf7a --- /dev/null +++ b/app/helpers/checkout_helper.rb @@ -0,0 +1,13 @@ +module CheckoutHelper + def checkout_adjustments_for_summary(order) + adjustments = order.adjustments.eligible + + adjustments.reject! { |a| a.originator_type == 'Spree::TaxRate' && a.amount == 0 } + + enterprise_fee_adjustments = adjustments.select { |a| a.originator_type == 'EnterpriseFee' } + adjustments.reject! { |a| a.originator_type == 'EnterpriseFee' } + adjustments << Spree::Adjustment.new(label: 'Distribution', amount: enterprise_fee_adjustments.sum(&:amount)) + + adjustments + end +end diff --git a/app/views/spree/checkout/_summary.html.erb b/app/views/spree/checkout/_summary.html.erb new file mode 100644 index 0000000000..125254177e --- /dev/null +++ b/app/views/spree/checkout/_summary.html.erb @@ -0,0 +1,36 @@ + +

<%= t(:order_summary) %>

+ + + + + + + + + + <% adjustments = checkout_adjustments_for_summary(order) %> + <% adjustments.each do |adjustment| %> + + + + + <% end %> + + + + + + + <% if order.price_adjustment_totals.present? %> + + <% @order.price_adjustment_totals.each do |label, total| %> + + + + + <% end %> + + <% end %> + +
<%= t(:item_total) %>:<%= order.display_item_total %>
<%= adjustment.label %>: <%= adjustment.display_amount.to_html %>
<%= t(:order_total) %>:<%= @order.display_total.to_html %>
<%= label %><%= total %>
diff --git a/spec/features/consumer/checkout_spec.rb b/spec/features/consumer/checkout_spec.rb index d85ae84ef4..8a84df61a9 100644 --- a/spec/features/consumer/checkout_spec.rb +++ b/spec/features/consumer/checkout_spec.rb @@ -89,6 +89,13 @@ feature %q{ ['Product distribution by Edible garden for Garlic', '$2.00', '']] page.should have_selector 'span.distribution-total', :text => '$3.00' + + # When I check out + click_link 'Checkout' + + # Then I should see a summary of my distribution charges + page.should have_selector 'tbody#summary-order-charges td', text: 'Distribution:' + page.should have_selector 'tbody#summary-order-charges td', text: '$3.00' end scenario "viewing delivery fees for order cycle distribution" do From 64c9d4254a39ed00da01635ff547eb89087fa89b Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 19 Aug 2013 17:28:00 +1000 Subject: [PATCH 29/30] Fix spec failures - enterprise fee calculator type change, distributor info rich text tests, summarised distribution fee at checkout --- spec/features/admin/enterprise_fees_spec.rb | 2 +- spec/features/consumer/checkout_spec.rb | 26 ++++----------------- spec/features/consumer/product_spec.rb | 15 +++--------- 3 files changed, 9 insertions(+), 34 deletions(-) diff --git a/spec/features/admin/enterprise_fees_spec.rb b/spec/features/admin/enterprise_fees_spec.rb index 71e262b70d..9d011ce493 100644 --- a/spec/features/admin/enterprise_fees_spec.rb +++ b/spec/features/admin/enterprise_fees_spec.rb @@ -27,7 +27,7 @@ feature %q{ page.should have_selector "#enterprise_fee_set_collection_attributes_0_enterprise_id" page.should have_selector "option[selected]", text: 'Packing' page.should have_selector "input[value='$0.50 / kg']" - page.should have_selector "option[selected]", text: 'Flat Rate (per order)' + page.should have_selector "option[selected]", text: 'Flat Rate (per item)' page.should have_selector "input[value='#{amount}']" end diff --git a/spec/features/consumer/checkout_spec.rb b/spec/features/consumer/checkout_spec.rb index 8a84df61a9..d474ea76bc 100644 --- a/spec/features/consumer/checkout_spec.rb +++ b/spec/features/consumer/checkout_spec.rb @@ -89,13 +89,6 @@ feature %q{ ['Product distribution by Edible garden for Garlic', '$2.00', '']] page.should have_selector 'span.distribution-total', :text => '$3.00' - - # When I check out - click_link 'Checkout' - - # Then I should see a summary of my distribution charges - page.should have_selector 'tbody#summary-order-charges td', text: 'Distribution:' - page.should have_selector 'tbody#summary-order-charges td', text: '$3.00' end scenario "viewing delivery fees for order cycle distribution" do @@ -295,18 +288,9 @@ feature %q{ # Distributor details should be displayed within('fieldset#shipping') do [@distributor.name, - @distributor.address.address1, - @distributor.address.city, - @distributor.address.zipcode, - @distributor.address.state_text, - @distributor.address.country.name, - @distributor.pickup_times, - @distributor.next_collection_at, - @distributor.contact, - @distributor.phone, - @distributor.email, - @distributor.description, - @distributor.website].each do |value| + @distributor.distributor_info, + @distributor.next_collection_at + ].each do |value| page.should have_content value end @@ -319,8 +303,8 @@ feature %q{ # -- Checkout: Delivery order_charges = page.all("tbody#summary-order-charges tr").map {|row| row.all('td').map(&:text)}.take(2) - order_charges.should == [["Product distribution by Edible garden for Fuji apples:", "$1.00"], - ["Product distribution by Edible garden for Garlic:", "$2.00"]] + order_charges.should == [["Shipping:", "$0.00"], + ["Distribution:", "$3.00"]] click_checkout_continue_button # -- Checkout: Payment diff --git a/spec/features/consumer/product_spec.rb b/spec/features/consumer/product_spec.rb index 4c983294b0..efd4fa1689 100644 --- a/spec/features/consumer/product_spec.rb +++ b/spec/features/consumer/product_spec.rb @@ -44,18 +44,9 @@ feature %q{ within '#product-distributor-details' do [d.name, - d.address.address1, - d.address.city, - d.address.zipcode, - d.address.state_text, - d.address.country.name, - d.pickup_times, - d.next_collection_at, - d.contact, - d.phone, - d.email, - d.description, - d.website].each do |value| + d.distributor_info, + d.next_collection_at + ].each do |value| page.should have_content value end From 5f01bd69011d70c895c3b807371aa641579ec664 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 19 Aug 2013 17:35:09 +1000 Subject: [PATCH 30/30] Generalise checkout email signoff message --- .../order_mailer/confirm_email_with_distributor_info.text.erb | 4 ++-- .../enterprises_distributor_info_rich_text_feature_spec.rb | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/chili/enterprises_distributor_info_rich_text_feature/app/views/spree/order_mailer/confirm_email_with_distributor_info.text.erb b/lib/chili/enterprises_distributor_info_rich_text_feature/app/views/spree/order_mailer/confirm_email_with_distributor_info.text.erb index e88f053e1a..aa3b59de8c 100644 --- a/lib/chili/enterprises_distributor_info_rich_text_feature/app/views/spree/order_mailer/confirm_email_with_distributor_info.text.erb +++ b/lib/chili/enterprises_distributor_info_rich_text_feature/app/views/spree/order_mailer/confirm_email_with_distributor_info.text.erb @@ -32,5 +32,5 @@ Collection / Delivery Details Thanks for your support. -Marcus and Angie, -Local Organics \ No newline at end of file +<%= @order.distributor.contact %>, +<%= @order.distributor.name %> diff --git a/spec/features/chili/enterprises_distributor_info_rich_text_feature_spec.rb b/spec/features/chili/enterprises_distributor_info_rich_text_feature_spec.rb index cd95945f8b..ae919c1a04 100644 --- a/spec/features/chili/enterprises_distributor_info_rich_text_feature_spec.rb +++ b/spec/features/chili/enterprises_distributor_info_rich_text_feature_spec.rb @@ -80,6 +80,7 @@ feature "enterprises distributor info as rich text" do complete_purchase_from_checkout_address_page wait_until { ActionMailer::Base.deliveries.length == 1 } email = ActionMailer::Base.deliveries.last + binding.pry email.body.should =~ /Chu ge sai yubi dan bisento tobi ashi yubi ge omote./ email.body.should =~ /Thursday 2nd May/ end