diff --git a/app/controllers/admin/enterprise_fees_controller.rb b/app/controllers/admin/enterprise_fees_controller.rb index 41450c82c9..2aa2a3aa30 100644 --- a/app/controllers/admin/enterprise_fees_controller.rb +++ b/app/controllers/admin/enterprise_fees_controller.rb @@ -28,11 +28,14 @@ module Admin product_distribution = ProductDistribution.where(:enterprise_fee_id => @object).first if product_distribution p = product_distribution.product - flash[:error] = "That enterprise fee cannot be deleted as it is referenced by a product distribution: #{p.id} - #{p.name}." + error = "That enterprise fee cannot be deleted as it is referenced by a product distribution: #{p.id} - #{p.name}." respond_with(@object) do |format| - format.html { redirect_to collection_url } - format.js { render text: flash[:error], status: 403 } + format.html do + flash[:error] = error + redirect_to collection_url + end + format.js { render text: error, status: 403 } end end end diff --git a/app/controllers/spree/admin/shipping_methods_controller_decorator.rb b/app/controllers/spree/admin/shipping_methods_controller_decorator.rb index 9077c7a41e..035f83e408 100644 --- a/app/controllers/spree/admin/shipping_methods_controller_decorator.rb +++ b/app/controllers/spree/admin/shipping_methods_controller_decorator.rb @@ -3,19 +3,18 @@ module Spree ShippingMethodsController.class_eval do before_filter :do_not_destroy_referenced_shipping_methods, :only => :destroy + # This method was originally written because ProductDistributions referenced shipping + # methods, and deleting a referenced shipping method would break all the reports that + # queried it. + # This has changed, and now all we're protecting is Orders, which is a spree resource. + # Do we really need to protect it ourselves? Does spree do this, or provide some means + # of preserving the shipping method information for past orders? def do_not_destroy_referenced_shipping_methods order = Order.where(:shipping_method_id => @object).first if order flash[:error] = "That shipping method cannot be deleted as it is referenced by an order: #{order.number}." redirect_to collection_url and return end - - product_distribution = ProductDistribution.where(:shipping_method_id => @object).first - if product_distribution - p = product_distribution.product - flash[:error] = "That shipping method cannot be deleted as it is referenced by a product distribution: #{p.id} - #{p.name}." - redirect_to collection_url and return - end end end end diff --git a/app/helpers/spree/orders_helper.rb b/app/helpers/spree/orders_helper.rb index 745d95cf02..d8430f723f 100644 --- a/app/helpers/spree/orders_helper.rb +++ b/app/helpers/spree/orders_helper.rb @@ -5,9 +5,9 @@ module Spree order.nil? || order.line_items.empty? end - def order_delivery_fee_subtotal(order, options={}) + def order_distribution_subtotal(order, options={}) options.reverse_merge! :format_as_currency => true - amount = OpenFoodWeb::Calculator::Itemwise.new.compute(order) + amount = order.adjustments.enterprise_fee.sum &:amount options.delete(:format_as_currency) ? number_to_currency(amount) : amount end diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index b325257bc6..4379788dfc 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -8,6 +8,7 @@ class Enterprise < ActiveRecord::Base belongs_to :address, :class_name => 'Spree::Address' has_many :product_distributions, :foreign_key => 'distributor_id', :dependent => :destroy has_many :distributed_products, :through => :product_distributions, :source => :product + has_many :enterprise_fees has_many :enterprise_roles has_many :users, through: :enterprise_roles diff --git a/app/models/open_food_web/calculator/itemwise.rb b/app/models/open_food_web/calculator/itemwise.rb deleted file mode 100644 index 76f54cc62c..0000000000 --- a/app/models/open_food_web/calculator/itemwise.rb +++ /dev/null @@ -1,13 +0,0 @@ -module OpenFoodWeb - class Calculator::Itemwise < Spree::Calculator - - def self.description - "Itemwise Shipping" - end - - def compute(object) - # Given an order, sum the shipping on each individual item - object.line_items.sum { |li| li.distribution_fee } || 0 - end - end -end diff --git a/app/models/product_distribution.rb b/app/models/product_distribution.rb index 44ce0d494b..fbad4e6dd9 100644 --- a/app/models/product_distribution.rb +++ b/app/models/product_distribution.rb @@ -1,7 +1,6 @@ class ProductDistribution < ActiveRecord::Base belongs_to :product, :class_name => 'Spree::Product' belongs_to :distributor, :class_name => 'Enterprise' - belongs_to :shipping_method, :class_name => 'Spree::ShippingMethod' belongs_to :enterprise_fee validates_presence_of :product_id, :on => :update diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb index f5879a0a13..67837feaea 100644 --- a/app/models/spree/ability_decorator.rb +++ b/app/models/spree/ability_decorator.rb @@ -4,7 +4,7 @@ class AbilityDecorator def initialize(user) if user.enterprises.count > 0 - #User can only access products that they are a supplier for + #Enterprise User can only access products that they are a supplier for can [:create], Spree::Product can [:admin, :read, :update, :bulk_edit, :clone, :destroy], Spree::Product do |product| user.enterprises.include? product.supplier @@ -28,6 +28,11 @@ class AbilityDecorator can [:admin, :index, :read, :create, :edit, :update, :fire], Spree::Adjustment can [:admin, :index, :read, :create, :edit, :update, :fire], Spree::ReturnAuthorization + #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| + user.enterprises.include? payment_method.distributor + end end end end diff --git a/app/models/spree/line_item_decorator.rb b/app/models/spree/line_item_decorator.rb index edca66c6fb..3b7ee891b8 100644 --- a/app/models/spree/line_item_decorator.rb +++ b/app/models/spree/line_item_decorator.rb @@ -1,28 +1,3 @@ Spree::LineItem.class_eval do attr_accessible :max_quantity - - before_create :set_distribution_fee - - - def update_distribution_fee_without_callbacks!(distributor) - set_distribution_fee(distributor) - update_column(:distribution_fee, distribution_fee) - update_column(:shipping_method_name, shipping_method_name) - end - - - private - - def shipping_method(distributor=nil) - distributor ||= self.order.distributor - self.product.shipping_method_for_distributor(distributor) - end - - def set_distribution_fee(distributor=nil) - order = OpenStruct.new :line_items => [self] - sm = shipping_method(distributor) - - self.distribution_fee = sm.compute_amount(order) - self.shipping_method_name = sm.name - end end diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index 7a01b408bb..b4d94eb45e 100644 --- a/app/models/spree/order_decorator.rb +++ b/app/models/spree/order_decorator.rb @@ -9,13 +9,11 @@ Spree::Order.class_eval do belongs_to :distributor, :class_name => 'Enterprise' belongs_to :cart - before_validation :shipping_address_from_distributor validate :products_available_from_new_distribution, :if => lambda { distributor_id_changed? || order_cycle_id_changed? } attr_accessible :order_cycle_id, :distributor_id before_validation :shipping_address_from_distributor - before_save :update_line_item_shipping_methods - after_create :set_default_shipping_method + # -- Scopes scope :managed_by, lambda { |user| @@ -43,12 +41,6 @@ Spree::Order.class_eval do save! end - def empty! - line_items.destroy_all - adjustments.destroy_all - set_default_shipping_method - end - def set_distributor!(distributor) self.distributor = distributor self.order_cycle = nil unless self.order_cycle.andand.has_distributor? distributor @@ -83,29 +75,14 @@ Spree::Order.class_eval do line_items.map { |li| li.variant } end - - private - - # On creation of the order (when the first item is added to the user's cart), set the - # shipping method to the first one available and create a shipment. - # order.create_shipment! creates an adjustment for the shipping cost on the order, - # which means that the customer can see their shipping cost at every step of the - # checkout process, not just after the delivery step. - # This is based on the assumption that there's only one shipping method visible to the user, - # which is a method using the itemwise shipping calculator. - def set_default_shipping_method - self.shipping_method = itemwise_shipping_method - if self.shipping_method - self.save! - self.create_shipment! - else - raise 'No default shipping method found' + # Show payment methods with no distributor or for this distributor + def available_payment_methods + @available_payment_methods ||= Spree::PaymentMethod.available(:front_end).select do |pm| + (self.distributor && (pm.distributor == self.distributor)) || pm.distributor == nil end end - def itemwise_shipping_method - Spree::ShippingMethod.all.find { |sm| sm.calculator.is_a? OpenFoodWeb::Calculator::Itemwise } - end + private def shipping_address_from_distributor if distributor @@ -119,13 +96,6 @@ Spree::Order.class_eval do end end - def update_line_item_shipping_methods - if %w(cart address delivery resumed).include? state - self.line_items.each { |li| li.update_distribution_fee_without_callbacks!(distributor) } - self.update! - end - end - def product_distribution_for(line_item) line_item.variant.product.product_distribution_for self.distributor end diff --git a/app/models/spree/payment_method_decorator.rb b/app/models/spree/payment_method_decorator.rb new file mode 100644 index 0000000000..f8759ac06b --- /dev/null +++ b/app/models/spree/payment_method_decorator.rb @@ -0,0 +1,21 @@ +Spree::PaymentMethod.class_eval do + belongs_to :distributor, :class_name => 'Enterprise' + + attr_accessible :distributor_id + + # -- Scopes + scope :in_distributor, lambda { |distributor| where(:distributor_id => distributor) } + + scope :managed_by, lambda { |user| + if user.has_spree_role?('admin') + scoped + else + where('distributor_id IN (?)', user.enterprises.map {|enterprise| enterprise.id }) + end + } +end + +# Ensure that all derived classes also allow distributor_id +Spree::Gateway.providers.each do |p| + p.attr_accessible :distributor_id +end \ No newline at end of file diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb index e11b0c2375..dc0b1bbf35 100644 --- a/app/models/spree/product_decorator.rb +++ b/app/models/spree/product_decorator.rb @@ -80,23 +80,6 @@ Spree::Product.class_eval do self.product_distributions.find_by_distributor_id(distributor) end - # This method is called on products that are distributed via order cycles, but at the time of - # writing (27-5-2013), order cycle fees were not implemented, so there's no defined result - # that this method should return. As a stopgap, we notify Bugsnag of the situation and return - # an undefined, but valid shipping method. When order cycle fees are implemented, this method - # should return the order cycle shipping method, or raise an exepction with the message, - # "This product is not available through that distributor". - def shipping_method_for_distributor(distributor) - distribution = product_distribution_for distributor - - unless distribution - Bugsnag.notify(Exception.new "No product distribution for product #{id} at distributor #{distributor.andand.id}. Perhaps this product is distributed via an order cycle? This is a warning that OrderCycle fees and shipping methods are not yet implemented, and the shipping fee charged is undefined until then.") - end - - distribution.andand.shipping_method || Spree::ShippingMethod.where("name != 'Delivery'").last - end - - # Build a product distribution for each distributor def build_product_distributions_for_user user Enterprise.is_distributor.managed_by(user).each do |distributor| diff --git a/app/overrides/add_distributor_fees_to_cart_form.rb b/app/overrides/add_distributor_fees_to_cart_form.rb deleted file mode 100644 index 883f4622ed..0000000000 --- a/app/overrides/add_distributor_fees_to_cart_form.rb +++ /dev/null @@ -1,5 +0,0 @@ -Deface::Override.new(:virtual_path => "spree/orders/edit", - :insert_after => "#empty-cart", - :partial => "spree/orders/distributor_fees", - :name => "add_distributor_fees_to_cart_form", - :original => 'b5a751777e66ccbd45d7f1b980ecd201af94cb5b') 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 new file mode 100644 index 0000000000..8ac8e7aad9 --- /dev/null +++ b/app/overrides/spree/admin/payment_methods/_form/add_distributor_to_admin_payment_method_edit.html.haml.deface @@ -0,0 +1,9 @@ +/ + insert_before '[data-hook="environment"]' + += f.field_container :distributor do + = 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"}) + = f.error_message_on :distributor diff --git a/app/views/spree/orders/_distributor_fees.html.haml b/app/views/spree/orders/_distributor_fees.html.haml deleted file mode 100644 index 015d4d77ee..0000000000 --- a/app/views/spree/orders/_distributor_fees.html.haml +++ /dev/null @@ -1,24 +0,0 @@ -#delivery-fees - %h2 Distribution Costs - = cms_snippet_content(Cms::Snippet.find_by_identifier('cart_distribution_costs')) - - %table#delivery - %thead - %tr - %th Item - %th Shipping Method - %th Delivery Fee - %tbody - - @order.line_items.each do |line_item| - - tr_class = cycle('', 'alt') - %tr{:class => tr_class} - %td - %h4= link_to line_item.product.name, product_path(line_item.product) - %td.item-shipping-method= line_item.shipping_method_name - %td.item-shipping-cost= number_to_currency line_item.distribution_fee - - .subtotal{'data-hook' => 'delivery_fees_subtotal'} - %h5 - Shipping total - \: - %span.order-total= order_delivery_fee_subtotal(@order) diff --git a/app/views/spree/orders/_inside_cart_form.html.haml b/app/views/spree/orders/_inside_cart_form.html.haml index 3ae636eb85..502546113f 100644 --- a/app/views/spree/orders/_inside_cart_form.html.haml +++ b/app/views/spree/orders/_inside_cart_form.html.haml @@ -8,9 +8,9 @@ \: %span.order-total.item-total= number_to_currency @order.item_total %h5 - Shipping total + Distribution total \: - %span.order-total.shipping-total= order_delivery_fee_subtotal(@order) + %span.order-total.distribution-total= order_distribution_subtotal(@order) %h4 Total \: diff --git a/config/application.rb b/config/application.rb index 3eb4699719..f0d852500a 100644 --- a/config/application.rb +++ b/config/application.rb @@ -26,7 +26,6 @@ module Openfoodweb # Register Spree calculators initializer "spree.register.calculators" do |app| - app.config.spree.calculators.shipping_methods << OpenFoodWeb::Calculator::Itemwise app.config.spree.calculators.shipping_methods << OpenFoodWeb::Calculator::Weight app.config.spree.calculators.enterprise_fees = [Spree::Calculator::FlatPercentItemTotal, diff --git a/db/migrate/20130807062657_add_distributor_id_to_payment_method.rb b/db/migrate/20130807062657_add_distributor_id_to_payment_method.rb new file mode 100644 index 0000000000..26ea241019 --- /dev/null +++ b/db/migrate/20130807062657_add_distributor_id_to_payment_method.rb @@ -0,0 +1,5 @@ +class AddDistributorIdToPaymentMethod < ActiveRecord::Migration + def change + add_column :spree_payment_methods, :distributor_id, :integer + end +end diff --git a/db/migrate/20130812233634_remove_shipping_method_from_product_distribution.rb b/db/migrate/20130812233634_remove_shipping_method_from_product_distribution.rb new file mode 100644 index 0000000000..cf3fee3020 --- /dev/null +++ b/db/migrate/20130812233634_remove_shipping_method_from_product_distribution.rb @@ -0,0 +1,9 @@ +class RemoveShippingMethodFromProductDistribution < ActiveRecord::Migration + def up + remove_column :product_distributions, :shipping_method_id + end + + def down + add_column :product_distributions, :shipping_method_id, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index ac3280a415..d9eff970c6 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20130809075103) do +ActiveRecord::Schema.define(:version => 20130812233634) do create_table "adjustment_metadata", :force => true do |t| t.integer "adjustment_id" @@ -229,7 +229,6 @@ ActiveRecord::Schema.define(:version => 20130809075103) do create_table "product_distributions", :force => true do |t| t.integer "product_id" t.integer "distributor_id" - t.integer "shipping_method_id" t.datetime "created_at" t.datetime "updated_at" t.integer "enterprise_fee_id" @@ -470,12 +469,13 @@ ActiveRecord::Schema.define(:version => 20130809075103) do t.string "type" t.string "name" t.text "description" - t.boolean "active", :default => true - t.string "environment", :default => "development" - t.datetime "created_at", :null => false - t.datetime "updated_at", :null => false + t.boolean "active", :default => true + t.string "environment", :default => "development" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.datetime "deleted_at" t.string "display_on" + t.integer "distributor_id" end create_table "spree_payments", :force => true do |t| diff --git a/db/suburb_seeds.rb b/db/suburb_seeds.rb index 054be282ce..8085551be5 100644 --- a/db/suburb_seeds.rb +++ b/db/suburb_seeds.rb @@ -11,7 +11,7 @@ module SuburbSeeder connection = ActiveRecord::Base.connection() - puts "-- Seeding australian suburbs" + puts "-- Seeding Australian suburbs" connection.execute(" INSERT INTO suburbs (postcode,name,state_id,latitude,longitude) VALUES (200,$$AUSTRALIAN NATIONAL UNIVERSITY$$,#{state_id_act},-35.277272,149.117136), @@ -16894,4 +16894,4 @@ module SuburbSeeder (6060,$$Yokine South$$,#{state_id_wa},-31.9097,115.849); ") end -end \ No newline at end of file +end diff --git a/lib/tasks/dev.rake b/lib/tasks/dev.rake index 495343fc27..5d9e2809e0 100644 --- a/lib/tasks/dev.rake +++ b/lib/tasks/dev.rake @@ -72,11 +72,7 @@ namespace :openfoodweb do FactoryGirl.create(:distributor_enterprise, :address => Spree::Address.find_by_zipcode("3040")) end - unless Spree::ShippingMethod.all.any? { |sm| sm.calculator.is_a? OpenFoodWeb::Calculator::Itemwise } - FactoryGirl.create(:itemwise_shipping_method) - end - - # -- Enterprise Users + # -- Enterprise users unless Spree::User.count > 1 puts "[#{task_name}] Seeding enterprise users" @@ -93,6 +89,13 @@ namespace :openfoodweb do puts " Distributor User created: #{u.email}/#{pw} (" + u.enterprise_roles.map{ |er| er.enterprise.name}.join(", ") + ")" end + # -- Enterprise fees + unless EnterpriseFee.count > 1 + Enterprise.is_distributor.each do |distributor| + FactoryGirl.create(:enterprise_fee, enterprise: distributor) + end + end + # -- Products unless Spree::Product.count > 0 puts "[#{task_name}] Seeding products" @@ -104,7 +107,7 @@ namespace :openfoodweb do ProductDistribution.create(:product => prod1, :distributor => Enterprise.is_distributor[0], - :shipping_method => Spree::ShippingMethod.first) + :enterprise_fee => Enterprise.is_distributor[0].enterprise_fees.first) prod2 = FactoryGirl.create(:product, @@ -114,7 +117,7 @@ namespace :openfoodweb do ProductDistribution.create(:product => prod2, :distributor => Enterprise.is_distributor[1], - :shipping_method => Spree::ShippingMethod.first) + :enterprise_fee => Enterprise.is_distributor[1].enterprise_fees.first) prod3 = FactoryGirl.create(:product, :name => 'Beef - 5kg Trays', :price => 50.00, @@ -123,7 +126,7 @@ namespace :openfoodweb do ProductDistribution.create(:product => prod3, :distributor => Enterprise.is_distributor[2], - :shipping_method => Spree::ShippingMethod.first) + :enterprise_fee => Enterprise.is_distributor[2].enterprise_fees.first) end end end diff --git a/spec/controllers/enterprises_controller_spec.rb b/spec/controllers/enterprises_controller_spec.rb index 93782ff21a..45d0374a84 100644 --- a/spec/controllers/enterprises_controller_spec.rb +++ b/spec/controllers/enterprises_controller_spec.rb @@ -1,11 +1,6 @@ require 'spec_helper' describe EnterprisesController do - before :each do - stub!(:before_save_new_order) - stub!(:after_save_new_order) - end - it "displays suppliers" do s = create(:supplier_enterprise) d = create(:distributor_enterprise) diff --git a/spec/factories.rb b/spec/factories.rb index b652afd596..1cdf062e9b 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -92,11 +92,6 @@ FactoryGirl.define do enterprise_fee { |pd| FactoryGirl.create(:enterprise_fee, enterprise: pd.distributor) } end - factory :itemwise_shipping_method, :parent => :shipping_method do - name 'Delivery' - calculator { FactoryGirl.build(:itemwise_calculator) } - end - factory :adjustment_metadata, :class => AdjustmentMetadata do adjustment { FactoryGirl.create(:adjustment) } enterprise { FactoryGirl.create(:distributor_enterprise) } @@ -105,9 +100,6 @@ FactoryGirl.define do enterprise_role 'distributor' end - factory :itemwise_calculator, :class => OpenFoodWeb::Calculator::Itemwise do - end - factory :weight_calculator, :class => OpenFoodWeb::Calculator::Weight do after(:build) { |c| c.set_preference(:per_kg, 0.5) } after(:create) { |c| c.set_preference(:per_kg, 0.5); c.save! } diff --git a/spec/features/admin/payment_method_spec.rb b/spec/features/admin/payment_method_spec.rb new file mode 100644 index 0000000000..67274ba5c8 --- /dev/null +++ b/spec/features/admin/payment_method_spec.rb @@ -0,0 +1,34 @@ +require "spec_helper" + +feature %q{ + As a Super Admin + I want to be able to set a distributor on each payment method +} do + include AuthenticationWorkflow + include WebHelper + + background do + @distributors = (1..3).map { create(:distributor_enterprise) } + end + + #Create and Edit uses same partial form + context "creating a payment method", js: true do + scenario "assigning a distributor to the payment method" do + login_to_admin_section + + click_link 'Configuration' + click_link 'Payment Methods' + click_link 'New Payment Method' + + fill_in 'payment_method_name', :with => 'Cheque payment method' + + select @distributors[0].name, :from => 'payment_method_distributor_id' + click_button 'Create' + + flash_message.should == 'Payment Method has been successfully created!' + + payment_method = Spree::PaymentMethod.find_by_name('Cheque payment method') + payment_method.distributor.should == @distributors[0] + end + end +end 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 ed56b57ed3..cd95945f8b 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 @@ -90,7 +90,7 @@ feature "enterprises distributor info as rich text" do zone = create(:zone) c = Spree::Country.find_by_name('Australia') Spree::ZoneMember.create(:zoneable => c, :zone => zone) - create(:itemwise_shipping_method, zone: zone) + create(:shipping_method, zone: zone) create(:payment_method, :description => 'Cheque payment method') end diff --git a/spec/features/consumer/add_to_cart_spec.rb b/spec/features/consumer/add_to_cart_spec.rb index c5e01c1efb..ffcc8c3e12 100644 --- a/spec/features/consumer/add_to_cart_spec.rb +++ b/spec/features/consumer/add_to_cart_spec.rb @@ -33,12 +33,12 @@ feature %q{ d2 = create(:distributor_enterprise) create(:product, :distributors => [d2]) p = create(:product, :price => 12.34) - create(:product_distribution, :product => p, :distributor => d1, :shipping_method => create(:shipping_method)) + create(:product_distribution, :product => p, :distributor => d1) - # ... with a flat rate shipping method of cost $1.23 - sm = p.product_distributions.first.shipping_method - sm.calculator.preferred_amount = 1.23 - sm.calculator.save! + # ... with a flat rate distribution fee of $1.23 + ef = p.product_distributions.first.enterprise_fee + ef.calculator = Spree::Calculator::FlatRate.new preferred_amount: 1.23 + ef.calculator.save! # When I choose a distributor visit spree.root_path @@ -51,15 +51,13 @@ feature %q{ # Then the correct totals should be displayed page.should have_selector 'span.item-total', :text => '$12.34' - page.should have_selector 'span.shipping-total', :text => '$1.23' + page.should have_selector 'span.distribution-total', :text => '$1.23' page.should have_selector 'span.grand-total', :text => '$13.57' - # And the item should be in my cart, with the distribution info set for the line item + # And the item should be in my cart order = Spree::Order.last line_item = order.line_items.first line_item.product.should == p - line_item.distribution_fee.should == 1.23 - line_item.shipping_method_name.should == p.product_distributions.first.shipping_method.name # And my order should have its distributor set to the chosen distributor order.distributor.should == d1 @@ -229,7 +227,7 @@ feature %q{ page.should have_selector 'span.item-total', :text => '$12.34' # TODO: Test these when order cycle fees is implemented - # page.should have_selector 'span.shipping-total', :text => '$1.23' + # page.should have_selector 'span.distribution-total', :text => '$1.23' # page.should have_selector 'span.grand-total', :text => '$13.57' # And the item should be in my cart diff --git a/spec/features/consumer/checkout_spec.rb b/spec/features/consumer/checkout_spec.rb index 873366ec0f..d9220c59aa 100644 --- a/spec/features/consumer/checkout_spec.rb +++ b/spec/features/consumer/checkout_spec.rb @@ -37,27 +37,30 @@ feature %q{ :country => Spree::Country.find_by_name('Australia')), :pickup_times => 'Tuesday, 4 PM') - @shipping_method_1 = create(:shipping_method, :name => 'Shipping Method One') - @shipping_method_1.calculator.set_preference :amount, 1 - @shipping_method_1.calculator.save! + @enterprise_fee_1 = create(:enterprise_fee, :name => 'Shipping Method One', :calculator => Spree::Calculator::FlatRate.new) + @enterprise_fee_1.calculator.set_preference :amount, 1 + @enterprise_fee_1.calculator.save! - @shipping_method_2 = create(:shipping_method, :name => 'Shipping Method Two') - @shipping_method_2.calculator.set_preference :amount, 2 - @shipping_method_2.calculator.save! + @enterprise_fee_2 = create(:enterprise_fee, :name => 'Shipping Method Two', :calculator => Spree::Calculator::FlatRate.new) + @enterprise_fee_2.calculator.set_preference :amount, 2 + @enterprise_fee_2.calculator.save! @product_1 = create(:product, :name => 'Fuji apples') - @product_1.product_distributions.create(:distributor => @distributor, :shipping_method => @shipping_method_1) - @product_1.product_distributions.create(:distributor => @distributor_alternative, :shipping_method => @shipping_method_1) + @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_2 = create(:product, :name => 'Garlic') - @product_2.product_distributions.create(:distributor => @distributor, :shipping_method => @shipping_method_2) - @product_2.product_distributions.create(:distributor => @distributor_alternative, :shipping_method => @shipping_method_2) + @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) @zone = create(:zone) c = Spree::Country.find_by_name('Australia') Spree::ZoneMember.create(:zoneable => c, :zone => @zone) - create(:itemwise_shipping_method, zone: @zone) - create(:payment_method, :description => 'Cheque payment method') + 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 @@ -80,30 +83,32 @@ feature %q{ # Fuji apples | Shipping Method One | $1.00 # # Subtotal: $3.00 - 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' + + # 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' end scenario "changing distributor updates delivery fees" do - # Given two distributors and shipping methods + # Given two distributors and enterprise fees d1 = create(:distributor_enterprise) d2 = create(:distributor_enterprise) - sm1 = create(:shipping_method) - sm1.calculator.set_preference :amount, 1.23; sm1.calculator.save! - sm2 = create(:free_shipping_method) - sm2.calculator.set_preference :amount, 2.34; sm2.calculator.save! + ef1 = create(:enterprise_fee, calculator: Spree::Calculator::FlatRate.new) + ef1.calculator.set_preference :amount, 1.23; ef1.calculator.save! + ef2 = create(:enterprise_fee, calculator: Spree::Calculator::FlatRate.new) + ef2.calculator.set_preference :amount, 2.34; ef2.calculator.save! # And two products both available from both distributors p1 = create(:product) - create(:product_distribution, product: p1, distributor: d1, shipping_method: sm1) - create(:product_distribution, product: p1, distributor: d2, shipping_method: sm2) + create(:product_distribution, product: p1, distributor: d1, enterprise_fee: ef1) + create(:product_distribution, product: p1, distributor: d2, enterprise_fee: ef2) p2 = create(:product) - create(:product_distribution, product: p2, distributor: d1, shipping_method: sm1) - create(:product_distribution, product: p2, distributor: d2, shipping_method: sm2) + create(:product_distribution, product: p2, distributor: d1, enterprise_fee: ef1) + create(:product_distribution, product: p2, distributor: d2, enterprise_fee: ef2) # When I add the first product to my cart with the first distributor #visit spree.root_path @@ -113,7 +118,7 @@ feature %q{ click_button 'Add To Cart' # Then I should see shipping costs for the first distributor - page.should have_selector 'span.shipping-total', text: '$1.23' + page.should have_selector 'span.distribution-total', text: '$1.23' # When add the second with the second distributor click_link 'Continue shopping' @@ -122,7 +127,7 @@ feature %q{ click_button 'Add To Cart' # Then I should see shipping costs for the second distributor - page.should have_selector 'span.shipping-total', text: '$4.68' + page.should have_selector 'span.distribution-total', text: '$4.68' end scenario "adding a product to cart after emptying cart shows correct delivery fees" do @@ -195,15 +200,21 @@ feature %q{ click_checkout_continue_button # -- Checkout: Delivery - page.should have_selector 'label', :text => "Delivery $3.00" + 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"]] click_checkout_continue_button # -- 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 # -- Checkout: Order complete page.should have_content('Your order has been processed successfully') - page.should have_content('Cheque payment method') + page.should have_content(@payment_method_all.description) # page.should have_content('Your order will be available on:') diff --git a/spec/helpers/products_helper_spec.rb b/spec/helpers/products_helper_spec.rb index a6e16bab55..040fad5db3 100644 --- a/spec/helpers/products_helper_spec.rb +++ b/spec/helpers/products_helper_spec.rb @@ -22,13 +22,8 @@ module Spree private def make_variant_stub(product_price, variant_price) - product = stub(:product) - product.stub(:price).and_return(product_price) - - variant = stub(:variant) - variant.stub(:product).and_return(product) - variant.stub(:price).and_return(variant_price) - + product = double(:product, price: product_price) + variant = double(:variant, product: product, price: variant_price) variant end end diff --git a/spec/lib/open_food_web/group_buy_report_spec.rb b/spec/lib/open_food_web/group_buy_report_spec.rb index ec23cab070..23bfc70957 100644 --- a/spec/lib/open_food_web/group_buy_report_spec.rb +++ b/spec/lib/open_food_web/group_buy_report_spec.rb @@ -13,8 +13,7 @@ module OpenFoodWeb @variant1 = create(:variant) @variant1.product.supplier = @supplier1 @variant1.product.save! - shipping_method = create(:shipping_method) - product_distribution = create(:product_distribution, :product => @variant1.product, :distributor => distributor, :shipping_method => create(:shipping_method)) + product_distribution = create(:product_distribution, :product => @variant1.product, :distributor => distributor) shipping_instructions = "pick up on thursday please!" order1 = create(:order, :distributor => distributor, :bill_address => bill_address, :special_instructions => shipping_instructions) @@ -29,7 +28,7 @@ module OpenFoodWeb @variant2 = create(:variant) @variant2.product.supplier = @supplier1 @variant2.product.save! - product_distribution = create(:product_distribution, :product => @variant2.product, :distributor => distributor, :shipping_method => create(:shipping_method)) + product_distribution = create(:product_distribution, :product => @variant2.product, :distributor => distributor) line_item22 = create(:line_item, :variant => @variant2, :order => order2) order2.line_items << line_item22 @@ -39,7 +38,7 @@ module OpenFoodWeb @variant3 = create(:variant, :weight => nil) @variant3.product.supplier = @supplier2 @variant3.product.save! - product_distribution = create(:product_distribution, :product => @variant3.product, :distributor => distributor, :shipping_method => create(:shipping_method)) + product_distribution = create(:product_distribution, :product => @variant3.product, :distributor => distributor) order3 = create(:order, :distributor => distributor, :bill_address => bill_address, :special_instructions => shipping_instructions) line_item31 = create(:line_item, :variant => @variant3, :order => order3) diff --git a/spec/lib/open_food_web/order_and_distributor_report_spec.rb b/spec/lib/open_food_web/order_and_distributor_report_spec.rb index cabeb5010f..d003254f7b 100644 --- a/spec/lib/open_food_web/order_and_distributor_report_spec.rb +++ b/spec/lib/open_food_web/order_and_distributor_report_spec.rb @@ -11,7 +11,7 @@ module OpenFoodWeb @distributor_address = create(:address, :address1 => "distributor address", :city => 'The Shire', :zipcode => "1234") @distributor = create(:distributor_enterprise, :address => @distributor_address) product = create(:product) - product_distribution = create(:product_distribution, :product => product, :distributor => @distributor, :shipping_method => create(:shipping_method)) + product_distribution = create(:product_distribution, :product => product, :distributor => @distributor) @shipping_instructions = "pick up on thursday please!" @order = create(:order, :distributor => @distributor, :bill_address => @bill_address, :special_instructions => @shipping_instructions) @payment_method = create(:payment_method) diff --git a/spec/lib/open_food_web/split_products_by_distribution_spec.rb b/spec/lib/open_food_web/split_products_by_distribution_spec.rb index afba1ebb9d..678f94c327 100644 --- a/spec/lib/open_food_web/split_products_by_distribution_spec.rb +++ b/spec/lib/open_food_web/split_products_by_distribution_spec.rb @@ -66,15 +66,8 @@ describe OpenFoodWeb::SplitProductsByDistribution do def build_product(distributor, in_distributor, order_cycle, in_order_cycle) product = double(:product) - if distributor - product.should_receive(:in_distributor?).any_number_of_times. - with(distributor).and_return(in_distributor) - end - - if order_cycle - product.should_receive(:in_order_cycle?).any_number_of_times. - with(order_cycle).and_return(in_order_cycle) - end + product.stub(:in_distributor?) { in_distributor } if distributor + product.stub(:in_order_cycle?) { in_order_cycle } if order_cycle product end diff --git a/spec/models/calculator/itemwise_spec.rb b/spec/models/calculator/itemwise_spec.rb deleted file mode 100644 index 781a285e80..0000000000 --- a/spec/models/calculator/itemwise_spec.rb +++ /dev/null @@ -1,20 +0,0 @@ -require 'spec_helper' - -describe OpenFoodWeb::Calculator::Itemwise do - it "computes the shipping cost for each line item in an order" do - line_item = double(:line_item) - line_item.should_receive(:distribution_fee).exactly(3).times.and_return(10) - - order = double(:order) - order.stub(:line_items).and_return([line_item]*3) - - subject.compute(order).should == 30 - end - - it "returns zero for an order with no items" do - order = double(:order) - order.stub(:line_items).and_return([]) - - subject.compute(order).should == 0 - end -end diff --git a/spec/models/line_item_spec.rb b/spec/models/line_item_spec.rb index 2b46b7d7f7..b238500b8e 100644 --- a/spec/models/line_item_spec.rb +++ b/spec/models/line_item_spec.rb @@ -2,53 +2,5 @@ require 'spec_helper' module Spree describe LineItem do - describe "computing shipping cost for its product" do - let(:shipping_method_10) do - sm = create(:shipping_method, name: 'SM10') - sm.calculator.set_preference :amount, 10 - sm - end - let(:shipping_method_20) do - sm = create(:shipping_method, name: 'SM20') - sm.calculator.set_preference :amount, 20 - sm - end - let(:distributor1) { create(:distributor_enterprise) } - let(:distributor2) { create(:distributor_enterprise) } - let!(:product) do - p = create(:product) - create(:product_distribution, product: p, distributor: distributor1, shipping_method: shipping_method_10) - create(:product_distribution, product: p, distributor: distributor2, shipping_method: shipping_method_20) - p - end - let(:order_d1) { create(:order, distributor: distributor1, state: 'complete') } - - it "sets distribution fee and shipping method name on creation" do - li = create(:line_item, order: order_d1, product: product) - li.distribution_fee.should == 10 - li.shipping_method_name.should == 'SM10' - end - - it "updates its distribution fee & shipping method name" do - li = create(:line_item, order: order_d1, product: product) - - li.update_distribution_fee_without_callbacks! distributor2 - - li.distribution_fee.should == 20 - li.shipping_method_name.should == 'SM20' - end - - describe "fetching its shipping method" do - it "fetches the shipping method for its product when distributor is supplied" do - li = create(:line_item, order: order_d1, product: product) - li.send(:shipping_method, distributor2).should == shipping_method_20 - end - - it "uses the order's distributor when no other distributor is provided" do - li = create(:line_item, order: order_d1, product: product) - li.send(:shipping_method).should == shipping_method_10 - end - end - end end end diff --git a/spec/models/order_spec.rb b/spec/models/order_spec.rb index 2cb7dc97e8..fe8051ee3c 100644 --- a/spec/models/order_spec.rb +++ b/spec/models/order_spec.rb @@ -1,19 +1,6 @@ require 'spec_helper' describe Spree::Order do - it "initialises a default shipping method after creation" do - shipping_method_regular = create(:shipping_method) - shipping_method_itemwise = create(:itemwise_shipping_method) - - subject.shipping_method.should be_nil - subject.adjustments.should be_empty - - subject.save! - - subject.shipping_method.should == shipping_method_itemwise - subject.adjustments.where(:label => "Shipping").should be_present - end - it "sets attributes on line items for variants" do d = create(:distributor_enterprise) p = create(:product, :distributors => [d]) diff --git a/spec/models/product_spec.rb b/spec/models/product_spec.rb index b6d9e5ac36..e60ebb0281 100644 --- a/spec/models/product_spec.rb +++ b/spec/models/product_spec.rb @@ -191,24 +191,6 @@ module Spree product_distribution = create(:product_distribution, product: product, distributor: distributor) product.product_distribution_for(distributor).should == product_distribution end - - it "finds the shipping method for a particular distributor" do - shipping_method = create(:shipping_method) - distributor = create(:distributor_enterprise) - product = create(:product) - product_distribution = create(:product_distribution, product: product, distributor: distributor, shipping_method: shipping_method) - product.shipping_method_for_distributor(distributor).should == shipping_method - end - - it "logs an error and returns an undefined shipping method if distributor is not found" do - distributor = create(:distributor_enterprise) - product = create(:product) - - Bugsnag.should_receive(:notify) - - product.shipping_method_for_distributor(distributor).should == - Spree::ShippingMethod.where("name != 'Delivery'").last - end end describe "membership" do diff --git a/spec/support/spree/init.rb b/spec/support/spree/init.rb index c60c7bc0d4..0007680de7 100644 --- a/spec/support/spree/init.rb +++ b/spec/support/spree/init.rb @@ -8,21 +8,3 @@ ProductDistribution.class_eval do self.enterprise_fee ||= EnterpriseFee.where(enterprise_id: distributor).first || FactoryGirl.create(:enterprise_fee, enterprise_id: distributor) end end - -Spree::Product.class_eval do - before_validation :init_shipping_method - - def init_shipping_method - FactoryGirl.create(:shipping_method) if Spree::ShippingMethod.where("name != 'Delivery'").empty? - end - -end - -# Create a default shipping method, required when creating orders -Spree::Order.class_eval do - before_create :init_shipping_method - - def init_shipping_method - FactoryGirl.create(:itemwise_shipping_method) unless Spree::ShippingMethod.all.any? { |sm| sm.calculator.is_a? OpenFoodWeb::Calculator::Itemwise } - end -end