From 85f18cfb1265b3113f6048985e46c9e9c62647bc Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 19 Sep 2012 14:27:11 +1000 Subject: [PATCH 01/20] Rob: Replace LineItem shipping_method method with relation --- app/models/spree/line_item_decorator.rb | 6 ++---- ...13335_add_shipping_method_to_line_items.rb | 19 +++++++++++++++++++ db/schema.rb | 11 ++++++----- 3 files changed, 27 insertions(+), 9 deletions(-) create mode 100644 db/migrate/20120919013335_add_shipping_method_to_line_items.rb diff --git a/app/models/spree/line_item_decorator.rb b/app/models/spree/line_item_decorator.rb index 6430aa9ad3..488fd68c03 100644 --- a/app/models/spree/line_item_decorator.rb +++ b/app/models/spree/line_item_decorator.rb @@ -1,9 +1,7 @@ Spree::LineItem.class_eval do - attr_accessible :max_quantity + belongs_to :shipping_method - def shipping_method - self.product.shipping_method_for_distributor(self.order.distributor) - end + attr_accessible :max_quantity def itemwise_shipping_cost order = OpenStruct.new :line_items => [self] diff --git a/db/migrate/20120919013335_add_shipping_method_to_line_items.rb b/db/migrate/20120919013335_add_shipping_method_to_line_items.rb new file mode 100644 index 0000000000..20897a0811 --- /dev/null +++ b/db/migrate/20120919013335_add_shipping_method_to_line_items.rb @@ -0,0 +1,19 @@ +class AddShippingMethodToLineItems < ActiveRecord::Migration + def up + add_column :spree_line_items, :shipping_method_id, :integer + + Spree::LineItem.all.each do |li| + begin + shipping_method = li.product.shipping_method_for_distributor(li.order.distributor) + rescue ArgumentError + shipping_method = Spree::ShippingMethod.find_by_name 'Producer Delivery' + end + + Spree::LineItem.update_all("shipping_method_id = #{shipping_method.id}", "id = #{li.id}") + end + end + + def down + remove_column :spree_line_items, :shipping_method_id + end +end diff --git a/db/schema.rb b/db/schema.rb index 577e55d47c..2a22eedd1f 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 => 20120913054660) do +ActiveRecord::Schema.define(:version => 20120919013335) do create_table "distributors", :force => true do |t| t.string "name" @@ -180,11 +180,12 @@ ActiveRecord::Schema.define(:version => 20120913054660) do create_table "spree_line_items", :force => true do |t| t.integer "order_id" t.integer "variant_id" - t.integer "quantity", :null => false - t.decimal "price", :precision => 8, :scale => 2, :null => false - t.datetime "created_at", :null => false - t.datetime "updated_at", :null => false + t.integer "quantity", :null => false + t.decimal "price", :precision => 8, :scale => 2, :null => false + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.integer "max_quantity" + t.integer "shipping_method_id" end add_index "spree_line_items", ["order_id"], :name => "index_line_items_on_order_id" From eaca160367b5106ad9c0dc46813c88feab0111f4 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 19 Sep 2012 15:01:50 +1000 Subject: [PATCH 02/20] Set line item shipping method when added to cart, update specs for changes to line item shipping method --- .../spree/orders_controller_decorator.rb | 12 +- spec/factories.rb | 4 + spec/models/line_item_spec.rb | 5 +- spec/requests/consumer/add_to_cart_spec.rb | 146 +++++++++--------- 4 files changed, 89 insertions(+), 78 deletions(-) diff --git a/app/controllers/spree/orders_controller_decorator.rb b/app/controllers/spree/orders_controller_decorator.rb index 2de1c82777..1385336087 100644 --- a/app/controllers/spree/orders_controller_decorator.rb +++ b/app/controllers/spree/orders_controller_decorator.rb @@ -1,6 +1,7 @@ Spree::OrdersController.class_eval do - before_filter :populate_order_distributor, :only => :populate - after_filter :populate_variant_attributes, :only => :populate + before_filter :populate_order_distributor, :only => :populate + after_filter :populate_variant_attributes, :only => :populate + after_filter :populate_line_item_shipping_method, :only => :populate def populate_order_distributor @distributor = params[:distributor_id].present? ? Spree::Distributor.find(params[:distributor_id]) : nil @@ -16,6 +17,13 @@ Spree::OrdersController.class_eval do end end + def populate_line_item_shipping_method + @order.line_items.where(:shipping_method_id => nil).each do |line_item| + line_item.shipping_method = line_item.product.shipping_method_for_distributor(line_item.order.distributor) + line_item.save! + end + end + def populate_variant_attributes if params.key? :variant_attributes params[:variant_attributes].each do |variant_id, attributes| diff --git a/spec/factories.rb b/spec/factories.rb index ec312e4858..47fbf6f3b8 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -52,6 +52,10 @@ FactoryGirl.modify do # end end + factory :line_item do + shipping_method { |li| li.product.shipping_method_for_distributor(li.order.distributor) } + end + factory :address do state { Spree::State.find_by_name 'Victoria' } country { Spree::Country.find_by_name 'Australia' || Spree::Country.first } diff --git a/spec/models/line_item_spec.rb b/spec/models/line_item_spec.rb index 8d19dcea5f..5b62b802f1 100644 --- a/spec/models/line_item_spec.rb +++ b/spec/models/line_item_spec.rb @@ -7,12 +7,9 @@ module Spree shipping_method = create(:shipping_method) shipping_method.calculator.set_preference :amount, 10 - product = double(:product) - product.should_receive(:shipping_method_for_distributor).and_return(shipping_method) - order = double(:order, :distributor => nil) - subject.stub(:product).and_return(product) + subject.stub(:shipping_method).and_return(shipping_method) subject.stub(:order).and_return(order) subject.itemwise_shipping_cost.should == 10 diff --git a/spec/requests/consumer/add_to_cart_spec.rb b/spec/requests/consumer/add_to_cart_spec.rb index 9db625c1fc..9172519292 100644 --- a/spec/requests/consumer/add_to_cart_spec.rb +++ b/spec/requests/consumer/add_to_cart_spec.rb @@ -27,7 +27,6 @@ feature %q{ #order.line_items.should be_empty end - scenario "adding the first product to the cart" do # Given a product and some distributors d1 = create(:distributor) @@ -44,9 +43,11 @@ feature %q{ select d1.name, :from => 'distributor_id' click_button 'Add To Cart' - # Then the item should be in my cart + # Then the item should be in my cart, with shipping method set for the line item order = Spree::Order.last - order.line_items.first.product.should == p + line_item = order.line_items.first + line_item.product.should == p + line_item.shipping_method.should == p.product_distributions.first.shipping_method # And my order should have its distributor set to the chosen distributor order.distributor.should == d1 @@ -130,76 +131,77 @@ feature %q{ end end - scenario "adding a product to the cart for a group buy" do - # Given a group buy product and a distributor - d = create(:distributor) - p = create(:product, :distributors => [d], :group_buy => true) + context "group buys" do + scenario "adding a product to the cart for a group buy" do + # Given a group buy product and a distributor + d = create(:distributor) + p = create(:product, :distributors => [d], :group_buy => true) - # When I add the item to my cart - visit spree.product_path p - select d.name, :from => 'distributor_id' - fill_in "variants_#{p.master.id}", :with => 2 - fill_in "variant_attributes_#{p.master.id}_max_quantity", :with => 3 - click_button 'Add To Cart' + # When I add the item to my cart + visit spree.product_path p + select d.name, :from => 'distributor_id' + fill_in "variants_#{p.master.id}", :with => 2 + fill_in "variant_attributes_#{p.master.id}_max_quantity", :with => 3 + click_button 'Add To Cart' - # Then the item should be in my cart with correct quantities - order = Spree::Order.last - li = order.line_items.first - li.product.should == p - li.quantity.should == 2 - li.max_quantity.should == 3 + # Then the item should be in my cart with correct quantities + order = Spree::Order.last + li = order.line_items.first + li.product.should == p + li.quantity.should == 2 + li.max_quantity.should == 3 + end + + scenario "adding a product with variants to the cart for a group buy" do + # Given a group buy product with variants and a distributor + d = create(:distributor) + p = create(:product, :distributors => [d], :group_buy => true) + create(:variant, :product => p) + + # When I add the item to my cart + visit spree.product_path p + select d.name, :from => 'distributor_id' + fill_in "quantity", :with => 2 + fill_in "max_quantity", :with => 3 + click_button 'Add To Cart' + + # Then the item should be in my cart with correct quantities + order = Spree::Order.last + li = order.line_items.first + li.product.should == p + li.quantity.should == 2 + li.max_quantity.should == 3 + end + + scenario "adding a product to cart that is not a group buy does not show max quantity field" do + # Given a group buy product and a distributor + d = create(:distributor) + p = create(:product, :distributors => [d], :group_buy => false) + + # When I view the add to cart form, there should not be a max quantity field + visit spree.product_path p + + page.should_not have_selector "#variant_attributes_#{p.master.id}_max_quantity" + end + + scenario "adding a product with a max quantity less than quantity results in max_quantity==quantity" do + # Given a group buy product and a distributor + d = create(:distributor) + p = create(:product, :distributors => [d], :group_buy => true) + + # When I add the item to my cart + visit spree.product_path p + select d.name, :from => 'distributor_id' + fill_in "variants_#{p.master.id}", :with => 2 + fill_in "variant_attributes_#{p.master.id}_max_quantity", :with => 1 + click_button 'Add To Cart' + + # Then the item should be in my cart with correct quantities + order = Spree::Order.last + li = order.line_items.first + li.product.should == p + li.quantity.should == 2 + li.max_quantity.should == 2 + end end - - scenario "adding a product with variants to the cart for a group buy" do - # Given a group buy product with variants and a distributor - d = create(:distributor) - p = create(:product, :distributors => [d], :group_buy => true) - create(:variant, :product => p) - - # When I add the item to my cart - visit spree.product_path p - select d.name, :from => 'distributor_id' - fill_in "quantity", :with => 2 - fill_in "max_quantity", :with => 3 - click_button 'Add To Cart' - - # Then the item should be in my cart with correct quantities - order = Spree::Order.last - li = order.line_items.first - li.product.should == p - li.quantity.should == 2 - li.max_quantity.should == 3 - end - - scenario "adding a product to cart that is not a group buy does not show max quantity field" do - # Given a group buy product and a distributor - d = create(:distributor) - p = create(:product, :distributors => [d], :group_buy => false) - - # When I view the add to cart form, there should not be a max quantity field - visit spree.product_path p - - page.should_not have_selector "#variant_attributes_#{p.master.id}_max_quantity" - end - - scenario "adding a product with a max quantity less than quantity results in max_quantity==quantity" do - # Given a group buy product and a distributor - d = create(:distributor) - p = create(:product, :distributors => [d], :group_buy => true) - - # When I add the item to my cart - visit spree.product_path p - select d.name, :from => 'distributor_id' - fill_in "variants_#{p.master.id}", :with => 2 - fill_in "variant_attributes_#{p.master.id}_max_quantity", :with => 1 - click_button 'Add To Cart' - - # Then the item should be in my cart with correct quantities - order = Spree::Order.last - li = order.line_items.first - li.product.should == p - li.quantity.should == 2 - li.max_quantity.should == 2 - end - end From 5402590d5f7a036ec2fef200e08a78cce68eeeae Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 19 Sep 2012 15:08:05 +1000 Subject: [PATCH 03/20] Add debug output on line item migration for lost data --- db/migrate/20120919013335_add_shipping_method_to_line_items.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/db/migrate/20120919013335_add_shipping_method_to_line_items.rb b/db/migrate/20120919013335_add_shipping_method_to_line_items.rb index 20897a0811..fd5974992d 100644 --- a/db/migrate/20120919013335_add_shipping_method_to_line_items.rb +++ b/db/migrate/20120919013335_add_shipping_method_to_line_items.rb @@ -7,6 +7,7 @@ class AddShippingMethodToLineItems < ActiveRecord::Migration shipping_method = li.product.shipping_method_for_distributor(li.order.distributor) rescue ArgumentError shipping_method = Spree::ShippingMethod.find_by_name 'Producer Delivery' + say "Line item #{li.id} does not have a valid shipping method, setting to '#{shipping_method.name}'" end Spree::LineItem.update_all("shipping_method_id = #{shipping_method.id}", "id = #{li.id}") From f1f9b5b2e8afd7d0184144366671c28ea10f60ff Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 19 Sep 2012 15:19:30 +1000 Subject: [PATCH 04/20] Make order and distributor report resilient to orders without distributors --- lib/open_food_web/order_and_distributor_report.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/open_food_web/order_and_distributor_report.rb b/lib/open_food_web/order_and_distributor_report.rb index eafd1886a9..a477fd0edd 100644 --- a/lib/open_food_web/order_and_distributor_report.rb +++ b/lib/open_food_web/order_and_distributor_report.rb @@ -22,7 +22,7 @@ module OpenFoodWeb order.bill_address.full_name, order.email, order.bill_address.phone, order.bill_address.city, line_item.product.sku, line_item.product.name, line_item.variant.options_text, line_item.quantity, line_item.max_quantity, line_item.price * line_item.quantity, line_item.itemwise_shipping_cost, order.payments.first.payment_method.andand.name, - order.distributor.name, order.distributor.pickup_address.address1, order.distributor.pickup_address.city, order.distributor.pickup_address.zipcode, order.special_instructions ] + order.distributor.andand.name, order.distributor.pickup_address.address1, order.distributor.pickup_address.city, order.distributor.pickup_address.zipcode, order.special_instructions ] end end order_and_distributor_details From 3894ce946deb69cdb583d0f489a4086336a11765 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 19 Sep 2012 15:46:56 +1000 Subject: [PATCH 05/20] Fix error on supplier page, add test to cover --- app/controllers/spree/suppliers_controller.rb | 2 ++ spec/requests/consumer/suppliers_spec.rb | 36 +++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 spec/requests/consumer/suppliers_spec.rb diff --git a/app/controllers/spree/suppliers_controller.rb b/app/controllers/spree/suppliers_controller.rb index 5591b30156..3b3f29cead 100644 --- a/app/controllers/spree/suppliers_controller.rb +++ b/app/controllers/spree/suppliers_controller.rb @@ -1,5 +1,7 @@ module Spree class SuppliersController < BaseController + helper 'spree/products' + def show options = {:supplier_id => params[:id]} options.merge(params.reject { |k,v| k == :id }) diff --git a/spec/requests/consumer/suppliers_spec.rb b/spec/requests/consumer/suppliers_spec.rb new file mode 100644 index 0000000000..a1eb1d40e8 --- /dev/null +++ b/spec/requests/consumer/suppliers_spec.rb @@ -0,0 +1,36 @@ +require 'spec_helper' + +feature %q{ + As a consumer + I want to see a list of products from a supplier + So that I can connect with them (and maybe buy stuff too) +} do + include AuthenticationWorkflow + include WebHelper + + scenario "viewing a list of suppliers" do + # Given some suppliers + s1 = create(:supplier) + s2 = create(:supplier) + s3 = create(:supplier) + + # When I go to the home page + visit spree.root_path + + # Then I should see a list containing all the suppliers + [s1, s2, s3].each { |s| page.should have_selector 'a', :text => s.name } + end + + scenario "viewing products provided by a supplier" do + # Given a supplier with a product + s = create(:supplier, :name => 'Murrnong') + p = create(:product, :supplier => s) + + # When I select the supplier + visit spree.root_path + click_link s.name + + # Then I should see the product + page.should have_content p.name + end +end From 757456dd57688ccd11e031d959be00128cc1e21a Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 20 Sep 2012 10:45:50 +1000 Subject: [PATCH 06/20] Set line item shipping method in callback so that Order#update shipping calcs run correctly --- app/controllers/spree/orders_controller_decorator.rb | 8 -------- app/models/spree/line_item_decorator.rb | 10 ++++++++++ spec/models/order_spec.rb | 8 ++++++-- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/app/controllers/spree/orders_controller_decorator.rb b/app/controllers/spree/orders_controller_decorator.rb index 1385336087..6e9b2dd68c 100644 --- a/app/controllers/spree/orders_controller_decorator.rb +++ b/app/controllers/spree/orders_controller_decorator.rb @@ -1,7 +1,6 @@ Spree::OrdersController.class_eval do before_filter :populate_order_distributor, :only => :populate after_filter :populate_variant_attributes, :only => :populate - after_filter :populate_line_item_shipping_method, :only => :populate def populate_order_distributor @distributor = params[:distributor_id].present? ? Spree::Distributor.find(params[:distributor_id]) : nil @@ -17,13 +16,6 @@ Spree::OrdersController.class_eval do end end - def populate_line_item_shipping_method - @order.line_items.where(:shipping_method_id => nil).each do |line_item| - line_item.shipping_method = line_item.product.shipping_method_for_distributor(line_item.order.distributor) - line_item.save! - end - end - def populate_variant_attributes if params.key? :variant_attributes params[:variant_attributes].each do |variant_id, attributes| diff --git a/app/models/spree/line_item_decorator.rb b/app/models/spree/line_item_decorator.rb index 488fd68c03..86a5f143ef 100644 --- a/app/models/spree/line_item_decorator.rb +++ b/app/models/spree/line_item_decorator.rb @@ -3,8 +3,18 @@ Spree::LineItem.class_eval do attr_accessible :max_quantity + before_create :set_itemwise_shipping_method + + def itemwise_shipping_cost order = OpenStruct.new :line_items => [self] shipping_method.compute_amount(order) end + + + private + + def set_itemwise_shipping_method + self.shipping_method = self.product.shipping_method_for_distributor(self.order.distributor) + end end diff --git a/spec/models/order_spec.rb b/spec/models/order_spec.rb index cfbab11597..aaa3d70c36 100644 --- a/spec/models/order_spec.rb +++ b/spec/models/order_spec.rb @@ -15,8 +15,11 @@ describe Spree::Order do end it "reveals permission for changing distributor" do + d = create(:distributor) + p = create(:product, :distributors => [d]) + + subject.distributor = d subject.save! - p = create(:product) subject.can_change_distributor?.should be_true subject.add_variant(p.master, 1) @@ -64,11 +67,12 @@ describe Spree::Order do end it "sets attributes on line items for variants" do - subject.save! d = create(:distributor) p = create(:product, :distributors => [d]) subject.distributor = d + subject.save! + subject.add_variant(p.master, 1) subject.set_variant_attributes(p.master, {'max_quantity' => '3'}) From 593f3887c86b5dfe6742a25c95861b9a4fba7f22 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 21 Sep 2012 12:29:06 +1000 Subject: [PATCH 07/20] Switch pry for pry-debugger --- Gemfile | 2 +- Gemfile.lock | 25 ++++++++++++++++++------- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/Gemfile b/Gemfile index 1029be0165..3328c8f2f0 100644 --- a/Gemfile +++ b/Gemfile @@ -47,7 +47,7 @@ group :test, :development do gem 'capybara' gem 'database_cleaner', '0.7.1', :require => false gem 'spork', '~> 1.0rc' - gem 'pry' + gem 'pry-debugger' gem 'awesome_print' end diff --git a/Gemfile.lock b/Gemfile.lock index 4fe7de7b48..02deb13906 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -110,7 +110,7 @@ GEM childprocess (0.3.2) ffi (~> 1.0.6) cocaine (0.3.0) - coderay (1.0.6) + coderay (1.0.7) coffee-rails (3.2.2) coffee-script (>= 2.2.0) railties (~> 3.2.0) @@ -118,7 +118,15 @@ GEM coffee-script-source execjs coffee-script-source (1.3.3) + columnize (0.3.6) database_cleaner (0.7.1) + debugger (1.1.4) + columnize (>= 0.3.1) + debugger-linecache (~> 1.1.1) + debugger-ruby_core_source (~> 1.1.3) + debugger-linecache (1.1.2) + debugger-ruby_core_source (>= 1.1.1) + debugger-ruby_core_source (1.1.3) deface (0.9.1) nokogiri (~> 1.5.0) rails (~> 3.1) @@ -163,7 +171,7 @@ GEM i18n (>= 0.4.0) mime-types (~> 1.16) treetop (~> 1.4.8) - method_source (0.7.1) + method_source (0.8) mime-types (1.19) money (3.7.1) i18n (~> 0.4) @@ -183,10 +191,13 @@ GEM polyamorous (0.5.0) activerecord (~> 3.0) polyglot (0.3.3) - pry (0.9.9.6) + pry (0.9.10) coderay (~> 1.0.5) - method_source (~> 0.7.1) - slop (>= 2.4.4, < 3) + method_source (~> 0.8) + slop (~> 3.3.1) + pry-debugger (0.2.0) + debugger (~> 1.1.3) + pry (~> 0.9.9) rabl (0.6.5) activesupport (>= 2.3.14) multi_json (~> 1.0) @@ -247,7 +258,7 @@ GEM rubyzip shoulda-matchers (1.1.0) activesupport (>= 3.0.0) - slop (2.4.4) + slop (3.3.3) spork (1.0.0rc3) spree (1.1.3) spree_api (= 1.1.3) @@ -338,7 +349,7 @@ DEPENDENCIES haml jquery-rails pg - pry + pry-debugger rails (= 3.2.8) rspec-rails sass-rails (~> 3.2.3) From a1b51024764d4adf2153c42b53e2d79fff15c094 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 21 Sep 2012 12:35:01 +1000 Subject: [PATCH 08/20] Fix checkout totals, also amend factories for valid test shipping method data --- app/models/spree/order_decorator.rb | 2 ++ .../spree/orders/_inside_cart_form.html.haml | 6 ++--- .../distributors_controller_spec.rb | 4 +++- spec/factories.rb | 15 ++++++++++++- spec/requests/consumer/add_to_cart_spec.rb | 22 ++++++++++++++----- 5 files changed, 39 insertions(+), 10 deletions(-) diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index e1127d131e..bced2e5a17 100644 --- a/app/models/spree/order_decorator.rb +++ b/app/models/spree/order_decorator.rb @@ -47,6 +47,8 @@ Spree::Order.class_eval do if self.shipping_method self.save! self.create_shipment! + else + raise 'No default shipping method found' end end diff --git a/app/views/spree/orders/_inside_cart_form.html.haml b/app/views/spree/orders/_inside_cart_form.html.haml index 18e3d6f0b3..117b0fed9e 100644 --- a/app/views/spree/orders/_inside_cart_form.html.haml +++ b/app/views/spree/orders/_inside_cart_form.html.haml @@ -6,15 +6,15 @@ %h5 Item total \: - %span.order-total= order_subtotal(@order) + %span.order-total.item-total= number_to_currency @order.item_total %h5 Shipping total \: - %span.order-total= order_delivery_fee_subtotal(@order) + %span.order-total.shipping-total= order_delivery_fee_subtotal(@order) %h4 Total \: - %span.order-total= number_to_currency(order_subtotal(@order, :format_as_currency => false) + order_delivery_fee_subtotal(@order, :format_as_currency => false)) + %span.order-total.grand-total= order_subtotal(@order) .links{'data-hook' => "cart_buttons"} = button_tag :class => 'primary', :id => 'update-button' do diff --git a/spec/controllers/distributors_controller_spec.rb b/spec/controllers/distributors_controller_spec.rb index 2d83e5ea45..3595b780e0 100644 --- a/spec/controllers/distributors_controller_spec.rb +++ b/spec/controllers/distributors_controller_spec.rb @@ -4,9 +4,11 @@ require 'spree/core/current_order' describe Spree::DistributorsController do include Spree::Core::CurrentOrder - before do + before :each do stub!(:before_save_new_order) stub!(:after_save_new_order) + + create(:itemwise_shipping_method) end diff --git a/spec/factories.rb b/spec/factories.rb index 47fbf6f3b8..0bc105d592 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -24,7 +24,7 @@ FactoryGirl.define do factory :product_distribution, :class => Spree::ProductDistribution do product { |pd| Spree::Product.first || FactoryGirl.create(:product) } distributor { |pd| Spree::Distributor.first || FactoryGirl.create(:distributor) } - shipping_method { |pd| Spree::ShippingMethod.first || FactoryGirl.create(:shipping_method) } + shipping_method { |pd| Spree::ShippingMethod.where("name != 'Delivery'").first || FactoryGirl.create(:shipping_method) } end factory :itemwise_shipping_method, :parent => :shipping_method do @@ -50,12 +50,25 @@ FactoryGirl.modify do # before(:create) do |product, evaluator| # product.product_distributions = [FactoryGirl.create(:product_distribution, :product => product)] # end + + # Do not create products distributed via the 'Delivery' shipping method + after(:create) do |product, evaluator| + pd = product.product_distributions.first + if pd.andand.shipping_method.andand.name == 'Delivery' + pd.shipping_method = Spree::ShippingMethod.where("name != 'Delivery'").first || FactoryGirl.create(:shipping_method) + pd.save! + end + end end factory :line_item do shipping_method { |li| li.product.shipping_method_for_distributor(li.order.distributor) } end + factory :shipping_method do + display_on '' + end + factory :address do state { Spree::State.find_by_name 'Victoria' } country { Spree::Country.find_by_name 'Australia' || Spree::Country.first } diff --git a/spec/requests/consumer/add_to_cart_spec.rb b/spec/requests/consumer/add_to_cart_spec.rb index 9172519292..d528866647 100644 --- a/spec/requests/consumer/add_to_cart_spec.rb +++ b/spec/requests/consumer/add_to_cart_spec.rb @@ -24,26 +24,38 @@ feature %q{ # And the product should not have been added to my cart Spree::Order.last.should be_nil - #order.line_items.should be_empty end scenario "adding the first product to the cart" do - # Given a product and some distributors + create(:itemwise_shipping_method) + + # Given a product, some distributors and a defined shipping cost d1 = create(:distributor) d2 = create(:distributor) - p = create(:product, :distributors => [d1]) create(:product, :distributors => [d2]) + p = create(:product, :price => 12.34) + create(:product_distribution, :product => p, :distributor => d1, :shipping_method => create(:shipping_method)) + + # ... 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! # When I choose a distributor visit spree.root_path click_link d2.name - # When I add an item to my cart from a different distributor + # And I add an item to my cart from a different distributor visit spree.product_path p select d1.name, :from => 'distributor_id' click_button 'Add To Cart' - # Then the item should be in my cart, with shipping method set for the line item + # 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.grand-total', :text => '$13.57' + + # And the item should be in my cart, with shipping method set for the line item order = Spree::Order.last line_item = order.line_items.first line_item.product.should == p From 62d6067c1a6171aaf048bbcbf2a1411532f6d5b2 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 21 Sep 2012 12:44:38 +1000 Subject: [PATCH 09/20] Do not truncate cents for product price update on add to cart JS --- app/assets/javascripts/store/products.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/store/products.js b/app/assets/javascripts/store/products.js index bd5aa329be..3e972161a6 100644 --- a/app/assets/javascripts/store/products.js +++ b/app/assets/javascripts/store/products.js @@ -28,7 +28,7 @@ function products_update_price_without_variant() { if(master_price == null) { // Store off the master price master_price = $("#product-price span.price").html(); - master_price = master_price.substr(1, master_price.length-2); + master_price = master_price.substring(1); $("#product-price span.price").data('master-price', master_price); } From 479c49752c1dc1a7f8b53cffbde763eae7765ac4 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 21 Sep 2012 15:32:19 +1000 Subject: [PATCH 10/20] Update spree_paypal_express, hopefully fixing 'Validation failed: State can't be blank' error --- Gemfile | 2 +- Gemfile.lock | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index 3328c8f2f0..af0d5d30dc 100644 --- a/Gemfile +++ b/Gemfile @@ -6,7 +6,7 @@ gem 'rails', '3.2.8' gem 'pg' gem 'spree', '1.1.3' gem 'spree_i18n', :git => 'git://github.com/spree/spree_i18n.git' -gem 'spree_paypal_express', :git => 'git://github.com/spree/spree_paypal_express.git' +gem 'spree_paypal_express', :git => 'git://github.com/spree/spree_paypal_express.git', :branch => '1-1-stable' gem 'spree_last_address', :git => 'git://github.com/dancinglightning/spree-last-address.git' diff --git a/Gemfile.lock b/Gemfile.lock index 02deb13906..2c8b08631d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -31,9 +31,11 @@ GIT GIT remote: git://github.com/spree/spree_paypal_express.git - revision: d48f4649506e876b88244dcb4e5a86250e4ef8f0 + revision: 2c61f1700ef853e3d4f39739793dc3308422e94b + branch: 1-1-stable specs: spree_paypal_express (1.1.0) + spree_auth (>= 1.0.0) spree_core (>= 1.0.0) GEM @@ -93,7 +95,7 @@ GEM nokogiri (>= 1.4.4) uuidtools (~> 2.1) bcrypt-ruby (3.0.1) - braintree (2.16.0) + braintree (2.17.0) builder (>= 2.0.0) bugsnag (1.1.2) httparty (~> 0.8.3) From 2bce9df25aec1c7409c91668c616b33b58b02935 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 21 Sep 2012 15:55:51 +1000 Subject: [PATCH 11/20] Track 1-1-stable on spree git, fixes order quantity change bug --- Gemfile | 2 +- Gemfile.lock | 108 +++++++++++++++++++++++++++------------------------ 2 files changed, 58 insertions(+), 52 deletions(-) diff --git a/Gemfile b/Gemfile index af0d5d30dc..64620156ae 100644 --- a/Gemfile +++ b/Gemfile @@ -4,7 +4,7 @@ ruby "1.9.3" gem 'rails', '3.2.8' gem 'pg' -gem 'spree', '1.1.3' +gem 'spree', :git => 'git://github.com/spree/spree.git', :branch => '1-1-stable' gem 'spree_i18n', :git => 'git://github.com/spree/spree_i18n.git' gem 'spree_paypal_express', :git => 'git://github.com/spree/spree_paypal_express.git', :branch => '1-1-stable' gem 'spree_last_address', :git => 'git://github.com/dancinglightning/spree-last-address.git' diff --git a/Gemfile.lock b/Gemfile.lock index 2c8b08631d..ab046c1c6b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -21,6 +21,54 @@ GIT aws-sdk (~> 1.3.4) spree_core (>= 0.70.0) +GIT + remote: git://github.com/spree/spree.git + revision: d66ffac9088d0d5d0b0d146f305044ce619f0464 + branch: 1-1-stable + specs: + spree (1.1.3) + spree_api (= 1.1.3) + spree_auth (= 1.1.3) + spree_cmd (= 1.1.3) + spree_core (= 1.1.3) + spree_dash (= 1.1.3) + spree_promo (= 1.1.3) + spree_sample (= 1.1.3) + spree_api (1.1.3) + rabl (= 0.6.5) + spree_auth (= 1.1.3) + spree_core (= 1.1.3) + spree_auth (1.1.3) + cancan (= 1.6.7) + devise (~> 2.0.0) + spree_core (= 1.1.3) + spree_cmd (1.1.3) + thor (>= 0.14.6) + spree_core (1.1.3) + activemerchant (= 1.28.0) + acts_as_list (= 0.1.4) + aws-sdk (~> 1.3.4) + deface (>= 0.9.0) + ffaker (~> 1.12.0) + highline (= 1.6.11) + jquery-rails (~> 2.0) + kaminari (= 0.13.0) + nested_set (= 1.7.0) + paperclip (~> 2.7) + rails (~> 3.2.8) + ransack (~> 0.6.0) + state_machine (= 1.1.2) + stringex (~> 1.3.2) + spree_dash (1.1.3) + httparty (~> 0.8.1) + spree_auth (= 1.1.3) + spree_core (= 1.1.3) + spree_promo (1.1.3) + spree_auth (= 1.1.3) + spree_core (= 1.1.3) + spree_sample (1.1.3) + spree_core (= 1.1.3) + GIT remote: git://github.com/spree/spree_i18n.git revision: a96bee02340e008e60549295a4f09e047fd2e628 @@ -57,14 +105,14 @@ GEM active_utils (1.0.5) activesupport (>= 2.3.11) i18n - activemerchant (1.20.4) + activemerchant (1.28.0) active_utils (>= 1.0.2) activesupport (>= 2.3.11) - braintree (>= 2.0.0) builder (>= 2.0.0) i18n json (>= 1.5.1) - money (<= 3.7.1) + money + nokogiri activemodel (3.2.8) activesupport (= 3.2.8) builder (~> 3.0.0) @@ -95,8 +143,6 @@ GEM nokogiri (>= 1.4.4) uuidtools (~> 2.1) bcrypt-ruby (3.0.1) - braintree (2.17.0) - builder (>= 2.0.0) bugsnag (1.1.2) httparty (~> 0.8.3) multi_json (~> 1.3.4) @@ -158,13 +204,14 @@ GEM multi_xml i18n (0.6.1) journey (1.0.4) - jquery-rails (2.0.3) + jquery-rails (2.1.2) railties (>= 3.1.0, < 5.0) thor (~> 0.14) json (1.7.5) - kaminari (0.14.1) + kaminari (0.13.0) actionpack (>= 3.0.0) activesupport (>= 3.0.0) + railties (>= 3.0.0) kgio (2.7.4) libv8 (3.3.10.4) libwebsocket (0.1.3) @@ -175,8 +222,9 @@ GEM treetop (~> 1.4.8) method_source (0.8) mime-types (1.19) - money (3.7.1) + money (5.0.0) i18n (~> 0.4) + json multi_json (1.3.6) multi_xml (0.5.1) nested_set (1.7.0) @@ -262,48 +310,6 @@ GEM activesupport (>= 3.0.0) slop (3.3.3) spork (1.0.0rc3) - spree (1.1.3) - spree_api (= 1.1.3) - spree_auth (= 1.1.3) - spree_cmd (= 1.1.3) - spree_core (= 1.1.3) - spree_dash (= 1.1.3) - spree_promo (= 1.1.3) - spree_sample (= 1.1.3) - spree_api (1.1.3) - rabl (= 0.6.5) - spree_auth (= 1.1.3) - spree_core (= 1.1.3) - spree_auth (1.1.3) - cancan (= 1.6.7) - devise (~> 2.0.0) - spree_core (= 1.1.3) - spree_cmd (1.1.3) - thor (>= 0.14.6) - spree_core (1.1.3) - activemerchant (= 1.20.4) - acts_as_list (= 0.1.4) - aws-sdk (~> 1.3.4) - deface (>= 0.9.0) - ffaker (~> 1.12.0) - highline (= 1.6.11) - jquery-rails (~> 2.0.0) - kaminari (>= 0.13.0) - nested_set (= 1.7.0) - paperclip (~> 2.7) - rails (~> 3.2.6) - ransack (~> 0.6.0) - state_machine (= 1.1.2) - stringex (~> 1.3.2) - spree_dash (1.1.3) - httparty (~> 0.8.1) - spree_auth (= 1.1.3) - spree_core (= 1.1.3) - spree_promo (1.1.3) - spree_auth (= 1.1.3) - spree_core (= 1.1.3) - spree_sample (1.1.3) - spree_core (= 1.1.3) sprockets (2.1.3) hike (~> 1.2) rack (~> 1.0) @@ -358,7 +364,7 @@ DEPENDENCIES shoulda-matchers simple_form! spork (~> 1.0rc) - spree (= 1.1.3) + spree! spree_heroku! spree_i18n! spree_last_address! From a8758f12719b8c27550972b22f45ae5be9e9fce1 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 21 Sep 2012 16:36:50 +1000 Subject: [PATCH 12/20] Fix OrdersHelper for upgraded spree --- .../spree/{orders_helper_decorator.rb => orders_helper.rb} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename app/helpers/spree/{orders_helper_decorator.rb => orders_helper.rb} (100%) diff --git a/app/helpers/spree/orders_helper_decorator.rb b/app/helpers/spree/orders_helper.rb similarity index 100% rename from app/helpers/spree/orders_helper_decorator.rb rename to app/helpers/spree/orders_helper.rb From bc408d8b6eefd3e40356d5e0825a19e061c77c7c Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Sat, 22 Sep 2012 09:20:27 +1000 Subject: [PATCH 13/20] Add jquery and jquery_ujs includes to admin/all.js, as these have been removed from spree/admin/spree_core.js --- app/assets/javascripts/admin/all.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/assets/javascripts/admin/all.js b/app/assets/javascripts/admin/all.js index daa358f585..226359330f 100644 --- a/app/assets/javascripts/admin/all.js +++ b/app/assets/javascripts/admin/all.js @@ -5,6 +5,8 @@ // the compiled file. // +//= require jquery +//= require jquery_ujs //= require admin/spree_core //= require admin/spree_auth //= require admin/spree_promo From 27b7d59f460fa3440a6c7fd90f3408929f0f415d Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Tue, 2 Oct 2012 09:44:51 +1000 Subject: [PATCH 14/20] BaseController does not attempt to merge incomplete and current orders when they have differing distributors --- .../spree/base_controller_decorator.rb | 20 +++++++++++++++++++ spec/controllers/home_controller_spec.rb | 18 +++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 app/controllers/spree/base_controller_decorator.rb diff --git a/app/controllers/spree/base_controller_decorator.rb b/app/controllers/spree/base_controller_decorator.rb new file mode 100644 index 0000000000..7ad171e0be --- /dev/null +++ b/app/controllers/spree/base_controller_decorator.rb @@ -0,0 +1,20 @@ +module Spree + BaseController.class_eval do + + # Override definition in spree/auth/app/controllers/spree/base_controller_decorator.rb + # Do not attempt to merge incomplete and current orders when they have differing distributors + def set_current_order + if current_user + if current_user.respond_to?(:last_incomplete_order) + last_incomplete_order = current_user.last_incomplete_order + if session[:order_id].nil? && last_incomplete_order + session[:order_id] = last_incomplete_order.id + elsif current_order && last_incomplete_order && current_order != last_incomplete_order && + (current_order.distributor.nil? || current_order.distributor == last_incomplete_order.distributor) + current_order.merge!(last_incomplete_order) + end + end + end + end + end +end diff --git a/spec/controllers/home_controller_spec.rb b/spec/controllers/home_controller_spec.rb index b2e35ef562..327787aa9c 100644 --- a/spec/controllers/home_controller_spec.rb +++ b/spec/controllers/home_controller_spec.rb @@ -25,4 +25,22 @@ describe Spree::HomeController do assigns(:products_local).should == [p1] assigns(:products_remote).should == [p2] end + + context "BaseController: merging incomplete orders" do + it "does not attempt to merge incomplete and current orders when they have differing distributors" do + incomplete_order = double(:order, distributor: 1) + current_order = double(:order, distributor: 2) + + user = double(:user, last_incomplete_order: incomplete_order) + controller.stub(:current_user).and_return(user) + controller.stub(:current_order).and_return(current_order) + + incomplete_order.should_receive(:merge!).never + current_order.should_receive(:merge!).never + + session[:order_id] = 123 + + spree_get :index + end + end end From afb8faa96f250492e6e8c25ccba6c184bec902b4 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Tue, 2 Oct 2012 11:44:50 +1000 Subject: [PATCH 15/20] Update zeus config --- zeus.json | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/zeus.json b/zeus.json index 5a02216dbf..2ef8ae0046 100644 --- a/zeus.json +++ b/zeus.json @@ -1,5 +1,5 @@ { - "command": "ruby -rzeus/rails -eZeus.go", + "command": "ruby -rubygems -rzeus/rails -eZeus.go", "plan": { "boot": { @@ -9,12 +9,12 @@ "runner": ["r"], "console": ["c"], "server": ["s"], - "generate": ["g"] + "generate": ["g"], + "dbconsole": [] }, "test_environment": { - "testtask": [], - "test_helper": {"testrb": []}, - "spec_helper": {"rspec": []} + "cucumber_environment": {"cucumber": []}, + "test_helper": {"test": ["rspec", "testrb"]} } } } From 3886ec39f78394930d40d7dd2dcbbf0f07d7c623 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Tue, 2 Oct 2012 11:47:55 +1000 Subject: [PATCH 16/20] Add web helper method to visit paths with delete method --- spec/support/request/web_helper.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/spec/support/request/web_helper.rb b/spec/support/request/web_helper.rb index 3c649b07c0..fd70915933 100644 --- a/spec/support/request/web_helper.rb +++ b/spec/support/request/web_helper.rb @@ -76,6 +76,11 @@ module WebHelper page.find(:xpath, "//div[@class=\"ui-dialog-buttonset\"]//span[contains(text(),\"#{button_content}\")]").click end + def visit_delete(url) + response = Capybara.current_session.driver.delete url + click_link 'redirected' if response.status == 302 + end + def trigger_manual_event(field_selector, event = 'change') page.execute_script("$('#{field_selector}').trigger('#{event}');") end From 2aa450b072705b2cb6200250e116f1bdc8364f12 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Tue, 2 Oct 2012 11:51:13 +1000 Subject: [PATCH 17/20] Do not delete shipping method referenced by an order --- .../shipping_methods_controller_decorator.rb | 15 +++++++++++++ spec/requests/admin/shipping_methods_spec.rb | 21 +++++++++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 app/controllers/spree/admin/shipping_methods_controller_decorator.rb create mode 100644 spec/requests/admin/shipping_methods_spec.rb diff --git a/app/controllers/spree/admin/shipping_methods_controller_decorator.rb b/app/controllers/spree/admin/shipping_methods_controller_decorator.rb new file mode 100644 index 0000000000..6e8309da5e --- /dev/null +++ b/app/controllers/spree/admin/shipping_methods_controller_decorator.rb @@ -0,0 +1,15 @@ +module Spree + module Admin + ShippingMethodsController.class_eval do + before_filter :do_not_destroy_referenced_shipping_methods, :only => :destroy + + 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 + end + end + end + end +end diff --git a/spec/requests/admin/shipping_methods_spec.rb b/spec/requests/admin/shipping_methods_spec.rb new file mode 100644 index 0000000000..b0355516c8 --- /dev/null +++ b/spec/requests/admin/shipping_methods_spec.rb @@ -0,0 +1,21 @@ +require 'spec_helper' + +feature 'shipping methods' do + include AuthenticationWorkflow + include WebHelper + + scenario "deleting a shipping method referenced by an order" do + login_to_admin_section + + sm = create(:shipping_method) + o = create(:order, shipping_method: sm) + + visit_delete spree.admin_shipping_method_path(sm) + + page.should have_content "That shipping method cannot be deleted as it is referenced by an order: #{o.number}." + Spree::ShippingMethod.find(sm.id).should_not be_nil + end + + scenario "deleting a shipping method referenced by a product distribution" + scenario "deleting a shipping method referenced by a line item" +end From 473b4e9adccaacd58ce1fe146f39294436845bf2 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Tue, 2 Oct 2012 11:54:18 +1000 Subject: [PATCH 18/20] Add positive case test --- spec/requests/admin/shipping_methods_spec.rb | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/spec/requests/admin/shipping_methods_spec.rb b/spec/requests/admin/shipping_methods_spec.rb index b0355516c8..5897257997 100644 --- a/spec/requests/admin/shipping_methods_spec.rb +++ b/spec/requests/admin/shipping_methods_spec.rb @@ -4,16 +4,25 @@ feature 'shipping methods' do include AuthenticationWorkflow include WebHelper - scenario "deleting a shipping method referenced by an order" do + before :each do login_to_admin_section + @sm = create(:shipping_method) + end - sm = create(:shipping_method) - o = create(:order, shipping_method: sm) + scenario "deleting a shipping method" do + visit_delete spree.admin_shipping_method_path(@sm) - visit_delete spree.admin_shipping_method_path(sm) + page.should have_content "Shipping method \"#{@sm.name}\" has been successfully removed!" + Spree::ShippingMethod.where(:id => @sm.id).should be_empty + end + + scenario "deleting a shipping method referenced by an order" do + o = create(:order, shipping_method: @sm) + + visit_delete spree.admin_shipping_method_path(@sm) page.should have_content "That shipping method cannot be deleted as it is referenced by an order: #{o.number}." - Spree::ShippingMethod.find(sm.id).should_not be_nil + Spree::ShippingMethod.find(@sm.id).should_not be_nil end scenario "deleting a shipping method referenced by a product distribution" From 7430fc9bddbdfb4f00f09c658176f545c6f21194 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Tue, 2 Oct 2012 11:58:18 +1000 Subject: [PATCH 19/20] Do not delete shipping method referenced by a product distribution --- .../admin/shipping_methods_controller_decorator.rb | 7 +++++++ spec/requests/admin/shipping_methods_spec.rb | 12 +++++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/app/controllers/spree/admin/shipping_methods_controller_decorator.rb b/app/controllers/spree/admin/shipping_methods_controller_decorator.rb index 6e8309da5e..aa18bf8bba 100644 --- a/app/controllers/spree/admin/shipping_methods_controller_decorator.rb +++ b/app/controllers/spree/admin/shipping_methods_controller_decorator.rb @@ -9,6 +9,13 @@ module Spree flash[:error] = "That shipping method cannot be deleted as it is referenced by an order: #{order.number}." redirect_to collection_url 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 + end end end end diff --git a/spec/requests/admin/shipping_methods_spec.rb b/spec/requests/admin/shipping_methods_spec.rb index 5897257997..11414c011b 100644 --- a/spec/requests/admin/shipping_methods_spec.rb +++ b/spec/requests/admin/shipping_methods_spec.rb @@ -25,6 +25,16 @@ feature 'shipping methods' do Spree::ShippingMethod.find(@sm.id).should_not be_nil end - scenario "deleting a shipping method referenced by a product distribution" + scenario "deleting a shipping method referenced by a product distribution" do + p = create(:product) + d = create(:distributor) + create(:product_distribution, product: p, distributor: d, shipping_method: @sm) + + visit_delete spree.admin_shipping_method_path(@sm) + + page.should have_content "That shipping method cannot be deleted as it is referenced by a product distribution: #{p.id} - #{p.name}." + Spree::ShippingMethod.find(@sm.id).should_not be_nil + end + scenario "deleting a shipping method referenced by a line item" end From 6d9257d9411fedb5fc1017431b8a5b906a6a251b Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Tue, 2 Oct 2012 12:15:09 +1000 Subject: [PATCH 20/20] Do not delete shipping method referenced by a line item --- .../shipping_methods_controller_decorator.rb | 10 ++++++++-- spec/requests/admin/shipping_methods_spec.rb | 20 ++++++++++++++++++- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/app/controllers/spree/admin/shipping_methods_controller_decorator.rb b/app/controllers/spree/admin/shipping_methods_controller_decorator.rb index aa18bf8bba..84af3ef896 100644 --- a/app/controllers/spree/admin/shipping_methods_controller_decorator.rb +++ b/app/controllers/spree/admin/shipping_methods_controller_decorator.rb @@ -7,14 +7,20 @@ module Spree 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 + 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 + redirect_to collection_url and return + end + + line_item = LineItem.where(:shipping_method_id => @object).first + if line_item + flash[:error] = "That shipping method cannot be deleted as it is referenced by a line item in order: #{line_item.order.number}." + redirect_to collection_url and return end end end diff --git a/spec/requests/admin/shipping_methods_spec.rb b/spec/requests/admin/shipping_methods_spec.rb index 11414c011b..a72cff3e3a 100644 --- a/spec/requests/admin/shipping_methods_spec.rb +++ b/spec/requests/admin/shipping_methods_spec.rb @@ -36,5 +36,23 @@ feature 'shipping methods' do Spree::ShippingMethod.find(@sm.id).should_not be_nil end - scenario "deleting a shipping method referenced by a line item" + scenario "deleting a shipping method referenced by a line item" do + sm2 = create(:shipping_method) + d = create(:distributor) + + p = create(:product) + create(:product_distribution, product: p, distributor: d, shipping_method: sm2) + + o = create(:order, distributor: d) + o.shipping_method = sm2 + o.save! + li = create(:line_item, order: o, product: p) + li.shipping_method = @sm + li.save! + + visit_delete spree.admin_shipping_method_path(@sm) + + page.should have_content "That shipping method cannot be deleted as it is referenced by a line item in order: #{o.number}." + Spree::ShippingMethod.find(@sm.id).should_not be_nil + end end