From 5e6c4de34b17fd79326ffadc27428b1ca3e39acd Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Sat, 23 Jun 2012 12:33:38 +1000 Subject: [PATCH 01/25] Leave a distributor --- app/controllers/spree/distributors_controller.rb | 8 ++++++++ app/views/spree/products/_source_sidebar.html.haml | 2 ++ config/routes.rb | 1 + spec/requests/consumer/distributors_spec.rb | 13 ++++++++++++- 4 files changed, 23 insertions(+), 1 deletion(-) diff --git a/app/controllers/spree/distributors_controller.rb b/app/controllers/spree/distributors_controller.rb index d3a3375ebf..f0e195b88b 100644 --- a/app/controllers/spree/distributors_controller.rb +++ b/app/controllers/spree/distributors_controller.rb @@ -18,5 +18,13 @@ module Spree redirect_back_or_default(root_path) end + + def deselect + order = current_order(true) + order.distributor = nil + order.save! + + redirect_back_or_default(root_path) + end end end diff --git a/app/views/spree/products/_source_sidebar.html.haml b/app/views/spree/products/_source_sidebar.html.haml index 19a9516c18..ff8e82c9ab 100644 --- a/app/views/spree/products/_source_sidebar.html.haml +++ b/app/views/spree/products/_source_sidebar.html.haml @@ -8,3 +8,5 @@ %ul.filter_choices - @distributors.each do |distributor| %li.nowrap= link_to distributor.name, select_distributor_path(distributor) + - if current_distributor + %li.nowrap= link_to 'Leave distributor', deselect_distributors_path diff --git a/config/routes.rb b/config/routes.rb index 93c3e9d19d..60cec94135 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -8,6 +8,7 @@ Spree::Core::Engine.routes.prepend do resources :suppliers resources :distributors do get :select, :on => :member + get :deselect, :on => :collection end namespace :admin do diff --git a/spec/requests/consumer/distributors_spec.rb b/spec/requests/consumer/distributors_spec.rb index 4540dc2f2f..24e5c44c3d 100644 --- a/spec/requests/consumer/distributors_spec.rb +++ b/spec/requests/consumer/distributors_spec.rb @@ -39,7 +39,18 @@ feature %q{ it "splits the product listing by local/remote distributor" - it "allows the user to leave the distributor" + it "allows the user to leave the distributor" do + # Given a distributor + d = create(:distributor, :name => 'Melb Uni Co-op') + + # When I select the distributor and then leave it + visit spree.root_path + click_link d.name + click_link 'Leave distributor' + + # Then I should have left the distributor + page.should_not have_selector '#current-distributor', :text => 'You are shopping at Melb Uni Co-op' + end context "viewing a product" do it "provides a choice of distributor when adding to cart" # Test product at remote distributor From 5411685c8313494d55578e8cb1ec1805a30dbabc Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Sun, 24 Jun 2012 07:36:56 +1000 Subject: [PATCH 02/25] Split products by local/remote distributor: logic --- .../spree/home_controller_decorator.rb | 10 +++++++ .../split_products_by_distributor.rb | 19 ++++++++++++ spec/controllers/home_controller_spec.rb | 28 +++++++++++++++++ .../split_products_by_distributor_spec.rb | 30 +++++++++++++++++++ spec/requests/consumer/distributors_spec.rb | 28 +++++++++++++---- 5 files changed, 109 insertions(+), 6 deletions(-) create mode 100644 app/controllers/spree/home_controller_decorator.rb create mode 100644 lib/open_food_web/split_products_by_distributor.rb create mode 100644 spec/controllers/home_controller_spec.rb create mode 100644 spec/lib/open_food_web/split_products_by_distributor_spec.rb diff --git a/app/controllers/spree/home_controller_decorator.rb b/app/controllers/spree/home_controller_decorator.rb new file mode 100644 index 0000000000..c5bb07c687 --- /dev/null +++ b/app/controllers/spree/home_controller_decorator.rb @@ -0,0 +1,10 @@ +require 'open_food_web/split_products_by_distributor' + +Spree::HomeController.class_eval do + include Spree::DistributorsHelper + include OpenFoodWeb::SplitProductsByDistributor + + respond_override :index => { :html => { :success => lambda { + @products, @products_local, @products_remote = split_products_by_distributor @products, current_distributor + } } } +end diff --git a/lib/open_food_web/split_products_by_distributor.rb b/lib/open_food_web/split_products_by_distributor.rb new file mode 100644 index 0000000000..37802a5860 --- /dev/null +++ b/lib/open_food_web/split_products_by_distributor.rb @@ -0,0 +1,19 @@ +module OpenFoodWeb + module SplitProductsByDistributor + + # If a distributor is provided, split the list of products into local (at that + # distributor) and remote (at another distributor). If a distributor is not + # provided, perform no split. + def split_products_by_distributor(products, distributor) + products_local = products_remote = nil + + if distributor + products_local = products.select { |p| p.distributors.include? distributor } + products_remote = products.reject { |p| p.distributors.include? distributor } + products = nil + end + + [products, products_local, products_remote] + end + end +end diff --git a/spec/controllers/home_controller_spec.rb b/spec/controllers/home_controller_spec.rb new file mode 100644 index 0000000000..b2e35ef562 --- /dev/null +++ b/spec/controllers/home_controller_spec.rb @@ -0,0 +1,28 @@ +require 'spec_helper' + +describe Spree::HomeController do + it "loads products" do + product = create(:product) + spree_get :index + assigns(:products).should == [product] + assigns(:products_local).should be_nil + assigns(:products_remote).should be_nil + end + + it "splits products by local/remote distributor when distributor is selected" do + # Given two distributors with a product under each + d1 = create(:distributor) + d2 = create(:distributor) + p1 = create(:product, :distributors => [d1]) + p2 = create(:product, :distributors => [d2]) + + # And the first distributor is selected + controller.stub(:current_distributor).and_return(d1) + + # When I fetch the home page, the products should be split by local/remote distributor + spree_get :index + assigns(:products).should be_nil + assigns(:products_local).should == [p1] + assigns(:products_remote).should == [p2] + end +end diff --git a/spec/lib/open_food_web/split_products_by_distributor_spec.rb b/spec/lib/open_food_web/split_products_by_distributor_spec.rb new file mode 100644 index 0000000000..4924ac1061 --- /dev/null +++ b/spec/lib/open_food_web/split_products_by_distributor_spec.rb @@ -0,0 +1,30 @@ +require 'spec_helper' + +describe OpenFoodWeb::SplitProductsByDistributor do + let(:products_splitter) { Class.new { include OpenFoodWeb::SplitProductsByDistributor } } + let(:subject) { products_splitter.new } + + + it "does nothing when no distributor is selected" do + orig_products = (1..3).map { |i| build(:product) } + + products, products_local, products_remote = subject.split_products_by_distributor orig_products, nil + + products.should == orig_products + products_local.should be_nil + products_remote.should be_nil + end + + it "splits products when a distributor is selected" do + d1 = build(:distributor) + d2 = build(:distributor) + orig_products = [build(:product, :distributors => [d1]), + build(:product, :distributors => [d2])] + + products, products_local, products_remote = subject.split_products_by_distributor orig_products, d1 + + products.should be_nil + products_local.should == [orig_products[0]] + products_remote.should == [orig_products[1]] + end +end diff --git a/spec/requests/consumer/distributors_spec.rb b/spec/requests/consumer/distributors_spec.rb index 24e5c44c3d..c6ef0cbd79 100644 --- a/spec/requests/consumer/distributors_spec.rb +++ b/spec/requests/consumer/distributors_spec.rb @@ -8,12 +8,10 @@ feature %q{ include AuthenticationWorkflow include WebHelper - background do + scenario "viewing a list of distributors" do # Given some distributors 3.times { create(:distributor) } - end - scenario "viewing a list of distributors" do # When I go to the home page visit spree.root_path @@ -37,7 +35,25 @@ feature %q{ page.should have_selector '#current-distributor', :text => 'You are shopping at Melb Uni Co-op' end - it "splits the product listing by local/remote distributor" + it "splits the product listing by local/remote distributor" do + # Given two distributors, with a product under each + d1 = create(:distributor) + d2 = create(:distributor) + p1 = create(:product, :distributors => [d1]) + p2 = create(:product, :distributors => [d2]) + + # When I select the first distributor + visit spree.root_path + click_link d1.name + + # Then I should see products split by local/remote distributor + page.should_not have_selector '#products' + page.should have_selector '#products-local', :text => p1.name + page.should have_selector '#products-remote', :text => p2.name + + # TODO: Also see on products page and taxon page + + end it "allows the user to leave the distributor" do # Given a distributor @@ -52,9 +68,9 @@ feature %q{ page.should_not have_selector '#current-distributor', :text => 'You are shopping at Melb Uni Co-op' end - context "viewing a product" do - it "provides a choice of distributor when adding to cart" # Test product at remote distributor + context "viewing a product, it provides a choice of distributor when adding to cart" do it "displays the local distributor as the default choice when available for the current product" + it "functions with remote distributors" end end From 32050bbba493d3140933d43f5cceec605ed4545a Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Sun, 24 Jun 2012 07:37:25 +1000 Subject: [PATCH 03/25] Split products by local/remote distributor: views and basic style --- .../stylesheets/store/openfoodweb.css.scss | 55 +++++++++++++++++++ .../split_products_by_distributor.rb | 4 ++ app/views/spree/shared/_products.html.erb | 31 +++++++++++ .../shared/_products_by_distributor.html.haml | 5 ++ 4 files changed, 95 insertions(+) create mode 100644 app/overrides/split_products_by_distributor.rb create mode 100644 app/views/spree/shared/_products.html.erb create mode 100644 app/views/spree/shared/_products_by_distributor.html.haml diff --git a/app/assets/stylesheets/store/openfoodweb.css.scss b/app/assets/stylesheets/store/openfoodweb.css.scss index 6d377df06a..89affd418f 100644 --- a/app/assets/stylesheets/store/openfoodweb.css.scss +++ b/app/assets/stylesheets/store/openfoodweb.css.scss @@ -34,6 +34,7 @@ nav#filters { } } + /* Style the product source on the product details page in the * same manner as the product properties table above it. */ @@ -43,3 +44,57 @@ nav#filters { #product-properties td, #product-source td { width: 50%; } + + +/* Apply Spree's ul#products style to ul.product-listing. When viewing products + * split by distributor, a separate product listing is displayed for local and + * remote products, so the #products id is removed to avoid its duplication. + */ +ul.product-listing { + &:after { + content: " "; + display: block; + clear: both; + visibility: hidden; + line-height: 0; + height: 0; + } + + li { + text-align: center; + font-weight: bold; + margin-bottom: 20px; + + a { + display: block; + + &.info { + height: 35px; + margin-top: 5px; + font-size: $product_list_name_font_size; + color: $product_link_text_color; + border-bottom: 1px solid lighten($body_text_color, 60); + overflow: hidden; + } + } + + .product-image { + border: 1px solid lighten($body_text_color, 60); + padding: 5px; + min-height: 110px; + background-color: $product_background_color; + + &:hover { + border-color: $link_text_color; + } + + } + + .price { + color: $link_text_color; + font-size: $product_list_price_font_size; + padding-top: 5px; + display: block; + } + } +} diff --git a/app/overrides/split_products_by_distributor.rb b/app/overrides/split_products_by_distributor.rb new file mode 100644 index 0000000000..7f78d1eb3a --- /dev/null +++ b/app/overrides/split_products_by_distributor.rb @@ -0,0 +1,4 @@ +Deface::Override.new(:virtual_path => "spree/home/index", + :replace => "[data-hook='homepage_products']", + :partial => "spree/shared/products_by_distributor", + :name => "products") diff --git a/app/views/spree/shared/_products.html.erb b/app/views/spree/shared/_products.html.erb new file mode 100644 index 0000000000..0d633e7415 --- /dev/null +++ b/app/views/spree/shared/_products.html.erb @@ -0,0 +1,31 @@ +<% + paginated_products = @searcher.retrieve_products if params.key?(:keywords) + paginated_products ||= products +%> +<% if products.empty? %> + <%= t(:no_products_found) %> +<% elsif params.key?(:keywords) %> +
<%= t(:search_results, :keywords => h(params[:keywords])) %>
+<% end %> + +<% if products.any? %> +
    + <% reset_cycle('default') %> + <% products.each do |product| %> + <% if Spree::Config[:show_zero_stock_products] || product.has_stock? %> +
  • " data-hook="products_list_item" itemscope itemtype="http://schema.org/Product"> +
    + <%= link_to small_image(product, :itemprop => "image"), product, :itemprop => 'url' %> +
    + <%= link_to truncate(product.name, :length => 50), product, :class => 'info', :itemprop => "name", :title => product.name %> + <%= number_to_currency product.price %> +
  • + <% end %> + <% end %> +
+<% end %> + +<% if paginated_products.respond_to?(:num_pages) + params.delete(:search) + params.delete(:taxon) +%><%= paginate paginated_products %><% end %> diff --git a/app/views/spree/shared/_products_by_distributor.html.haml b/app/views/spree/shared/_products_by_distributor.html.haml new file mode 100644 index 0000000000..86ae4fda0f --- /dev/null +++ b/app/views/spree/shared/_products_by_distributor.html.haml @@ -0,0 +1,5 @@ +- if @products + #products= render 'spree/shared/products', :products => @products +- else + #products-local= render 'spree/shared/products', :products => @products_local + #products-remote= render 'spree/shared/products', :products => @products_remote From ebb624bdc3c40d989f2027da4b15876cb9cc9519 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Sun, 24 Jun 2012 09:10:25 +1000 Subject: [PATCH 04/25] Reload application files, routes and factories each spork run --- Gemfile | 2 +- config/initializers/spree.rb | 2 +- spec/{support => }/factories.rb | 0 spec/spec_helper.rb | 27 +++++++++++++++++---------- 4 files changed, 19 insertions(+), 12 deletions(-) rename spec/{support => }/factories.rb (100%) diff --git a/Gemfile b/Gemfile index ee3e7cf252..fda000b35c 100644 --- a/Gemfile +++ b/Gemfile @@ -50,7 +50,7 @@ group :test, :development do gem 'turn', '~> 0.8.3', :require => false gem 'rspec-rails' gem 'shoulda-matchers' - gem 'factory_girl_rails' + gem 'factory_girl_rails', :require => false gem 'faker' gem 'capybara' gem 'database_cleaner', '0.7.1', :require => false diff --git a/config/initializers/spree.rb b/config/initializers/spree.rb index 287475382f..09c05769e1 100644 --- a/config/initializers/spree.rb +++ b/config/initializers/spree.rb @@ -16,7 +16,7 @@ Spree.config do |config| # config.shipping_instructions = true config.checkout_zone = 'Australia' config.address_requires_state = true - config.default_country_id = 12 # This should be Australia, see:spree/core/db/default/spree/countries.yml + config.default_country_id = 12 # This should be Australia, see: spree/core/db/default/spree/countries.yml config.searcher_class = OpenFoodWeb::Searcher end diff --git a/spec/support/factories.rb b/spec/factories.rb similarity index 100% rename from spec/support/factories.rb rename to spec/factories.rb diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 77abe0c2ef..13c0c9af75 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -3,15 +3,15 @@ require 'spork' #uncomment the following line to use spork with the debugger #require 'spork/ext/ruby-debug' -ENV["RAILS_ENV"] ||= 'test' -require File.expand_path("../../config/environment", __FILE__) -require 'rspec/rails' -require 'rspec/autorun' -require 'capybara' -require 'database_cleaner' - Spork.prefork do + ENV["RAILS_ENV"] ||= 'test' + require File.expand_path("../../config/environment", __FILE__) + require 'rspec/rails' + require 'rspec/autorun' + require 'capybara' + require 'database_cleaner' + # Requires supporting ruby files with custom matchers and macros, etc, # in spec/support/ and its subdirectories. Dir[Rails.root.join("spec/support/**/*.rb")].each {|f| require f} @@ -63,11 +63,18 @@ Spork.prefork do config.include Spree::UrlHelpers config.include Spree::Core::TestingSupport::ControllerRequests, :type => :controller config.include Devise::TestHelpers, :type => :controller - config.include FactoryGirl::Syntax::Methods end end Spork.each_run do - # Dir["#{File.dirname(__FILE__)}/../app/**/*.rb"].each {|f| load f} - # Rails.application.reload_routes! + Dir["#{File.dirname(__FILE__)}/../app/**/*.rb"].each {|f| load f} + Dir["#{File.dirname(__FILE__)}/../lib/**/*.rb"].each {|f| load f} + + Rails.application.reload_routes! + + require 'factory_girl_rails' + + RSpec.configure do |config| + config.include FactoryGirl::Syntax::Methods + end end From 34f95ffb4adc6eaab5ee673bc06eb03b6a62f5e5 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Sun, 24 Jun 2012 09:21:44 +1000 Subject: [PATCH 05/25] Split products by local/remote distributor: apply to products, search results and taxon pages --- .../spree/products_controller_decorator.rb | 10 ++++++++ .../spree/taxons_controller_decorator.rb | 10 ++++++++ .../split_products_by_distributor.rb | 17 ++++++++++++- .../shared/_products_by_distributor.html.haml | 6 ++--- spec/requests/consumer/distributors_spec.rb | 25 +++++++++++++------ 5 files changed, 56 insertions(+), 12 deletions(-) create mode 100644 app/controllers/spree/products_controller_decorator.rb create mode 100644 app/controllers/spree/taxons_controller_decorator.rb diff --git a/app/controllers/spree/products_controller_decorator.rb b/app/controllers/spree/products_controller_decorator.rb new file mode 100644 index 0000000000..aa9e6db748 --- /dev/null +++ b/app/controllers/spree/products_controller_decorator.rb @@ -0,0 +1,10 @@ +require 'open_food_web/split_products_by_distributor' + +Spree::ProductsController.class_eval do + include Spree::DistributorsHelper + include OpenFoodWeb::SplitProductsByDistributor + + respond_override :index => { :html => { :success => lambda { + @products, @products_local, @products_remote = split_products_by_distributor @products, current_distributor + } } } +end diff --git a/app/controllers/spree/taxons_controller_decorator.rb b/app/controllers/spree/taxons_controller_decorator.rb new file mode 100644 index 0000000000..a6aacc38a1 --- /dev/null +++ b/app/controllers/spree/taxons_controller_decorator.rb @@ -0,0 +1,10 @@ +require 'open_food_web/split_products_by_distributor' + +Spree::TaxonsController.class_eval do + include Spree::DistributorsHelper + include OpenFoodWeb::SplitProductsByDistributor + + respond_override :show => { :html => { :success => lambda { + @products, @products_local, @products_remote = split_products_by_distributor @products, current_distributor + } } } +end diff --git a/app/overrides/split_products_by_distributor.rb b/app/overrides/split_products_by_distributor.rb index 7f78d1eb3a..7e57b1909a 100644 --- a/app/overrides/split_products_by_distributor.rb +++ b/app/overrides/split_products_by_distributor.rb @@ -1,4 +1,19 @@ Deface::Override.new(:virtual_path => "spree/home/index", :replace => "[data-hook='homepage_products']", :partial => "spree/shared/products_by_distributor", - :name => "products") + :name => "products_home") + +Deface::Override.new(:virtual_path => "spree/products/index", + :replace => "[data-hook='homepage_products']", + :partial => "spree/shared/products_by_distributor", + :name => "products_products") + +Deface::Override.new(:virtual_path => "spree/products/index", + :replace => "[data-hook='search_results']", + :partial => "spree/shared/products_by_distributor", + :name => "products_search") + +Deface::Override.new(:virtual_path => "spree/taxons/show", + :replace => "[data-hook='taxon_products']", + :partial => "spree/shared/products_by_distributor", + :name => "products_taxon") diff --git a/app/views/spree/shared/_products_by_distributor.html.haml b/app/views/spree/shared/_products_by_distributor.html.haml index 86ae4fda0f..96b0f1973d 100644 --- a/app/views/spree/shared/_products_by_distributor.html.haml +++ b/app/views/spree/shared/_products_by_distributor.html.haml @@ -1,5 +1,5 @@ - if @products - #products= render 'spree/shared/products', :products => @products + #products= render 'spree/shared/products', :products => @products, :taxon => @taxon - else - #products-local= render 'spree/shared/products', :products => @products_local - #products-remote= render 'spree/shared/products', :products => @products_remote + #products-local= render 'spree/shared/products', :products => @products_local, :taxon => @taxon + #products-remote= render 'spree/shared/products', :products => @products_remote, :taxon => @taxon diff --git a/spec/requests/consumer/distributors_spec.rb b/spec/requests/consumer/distributors_spec.rb index c6ef0cbd79..c1b3747ba8 100644 --- a/spec/requests/consumer/distributors_spec.rb +++ b/spec/requests/consumer/distributors_spec.rb @@ -36,23 +36,32 @@ feature %q{ end it "splits the product listing by local/remote distributor" do - # Given two distributors, with a product under each + # Given two distributors, with a product under each, and each product under a taxon + taxonomy = Spree::Taxonomy.find_by_name('Products') || create(:taxonomy, :name => 'Products') + taxonomy_root = taxonomy.root + taxon = create(:taxon, :name => 'Taxon one', :parent_id => taxonomy_root.id) d1 = create(:distributor) d2 = create(:distributor) - p1 = create(:product, :distributors => [d1]) - p2 = create(:product, :distributors => [d2]) + p1 = create(:product, :distributors => [d1], :taxons => [taxon]) + p2 = create(:product, :distributors => [d2], :taxons => [taxon]) # When I select the first distributor visit spree.root_path click_link d1.name # Then I should see products split by local/remote distributor - page.should_not have_selector '#products' - page.should have_selector '#products-local', :text => p1.name - page.should have_selector '#products-remote', :text => p2.name - - # TODO: Also see on products page and taxon page + # on the home page, the products page, the search results page and the taxon page + [spree.root_path, + spree.products_path, + spree.products_path(:keywords => 'Product'), + spree.nested_taxons_path(taxon.permalink) + ].each do |path| + visit path + page.should_not have_selector '#products' + page.should have_selector '#products-local', :text => p1.name + page.should have_selector '#products-remote', :text => p2.name + end end it "allows the user to leave the distributor" do From 9be5dc41b98062efe5047f78ddad8cddbb3c04db Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Sun, 24 Jun 2012 09:41:33 +1000 Subject: [PATCH 06/25] Split products by local/remote distributor: style --- app/assets/stylesheets/store/openfoodweb.css.scss | 9 +++++++++ .../spree/shared/_products_by_distributor.html.haml | 8 ++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/store/openfoodweb.css.scss b/app/assets/stylesheets/store/openfoodweb.css.scss index 89affd418f..3b0c18b6cb 100644 --- a/app/assets/stylesheets/store/openfoodweb.css.scss +++ b/app/assets/stylesheets/store/openfoodweb.css.scss @@ -98,3 +98,12 @@ ul.product-listing { } } } + + +/* Highlight local products in distributor-split product listings */ +#products-local ul { + margin-bottom: 1em; + padding: 10px; + border-radius: 10px; + background-color: #def; +} diff --git a/app/views/spree/shared/_products_by_distributor.html.haml b/app/views/spree/shared/_products_by_distributor.html.haml index 96b0f1973d..c7f2200ae2 100644 --- a/app/views/spree/shared/_products_by_distributor.html.haml +++ b/app/views/spree/shared/_products_by_distributor.html.haml @@ -1,5 +1,9 @@ - if @products #products= render 'spree/shared/products', :products => @products, :taxon => @taxon - else - #products-local= render 'spree/shared/products', :products => @products_local, :taxon => @taxon - #products-remote= render 'spree/shared/products', :products => @products_remote, :taxon => @taxon + #products-local + %h2= "Products at #{current_distributor.name}" + = render 'spree/shared/products', :products => @products_local, :taxon => @taxon + #products-remote + %h2 Products found elsewhere + = render 'spree/shared/products', :products => @products_remote, :taxon => @taxon From a91722a5933bf6c7bafcd07cf049325cf059555f Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Sun, 24 Jun 2012 11:28:06 +1000 Subject: [PATCH 07/25] Display distributor choice on add to cart form --- .../spree/products_controller_decorator.rb | 8 ++++ app/models/spree/distributor.rb | 2 + .../add_distributor_to_add_to_cart_form.rb | 4 ++ .../spree/products/_add_to_cart.html.haml | 12 ++++++ spec/requests/consumer/distributors_spec.rb | 37 ++++++++----------- 5 files changed, 41 insertions(+), 22 deletions(-) create mode 100644 app/overrides/add_distributor_to_add_to_cart_form.rb create mode 100644 app/views/spree/products/_add_to_cart.html.haml diff --git a/app/controllers/spree/products_controller_decorator.rb b/app/controllers/spree/products_controller_decorator.rb index aa9e6db748..c25ca0f02a 100644 --- a/app/controllers/spree/products_controller_decorator.rb +++ b/app/controllers/spree/products_controller_decorator.rb @@ -4,7 +4,15 @@ Spree::ProductsController.class_eval do include Spree::DistributorsHelper include OpenFoodWeb::SplitProductsByDistributor + before_filter :load_distributors, :only => :show + respond_override :index => { :html => { :success => lambda { @products, @products_local, @products_remote = split_products_by_distributor @products, current_distributor } } } + + + def load_distributors + @distributors = Spree::Distributor.by_name + end + end diff --git a/app/models/spree/distributor.rb b/app/models/spree/distributor.rb index 29a17d7293..083e3fc2fc 100644 --- a/app/models/spree/distributor.rb +++ b/app/models/spree/distributor.rb @@ -8,6 +8,8 @@ module Spree validates :name, :pickup_address, :country_id, :state_id, :city, :post_code, :presence => true + scope :by_name, order('name') + after_initialize :initialize_country def initialize_country diff --git a/app/overrides/add_distributor_to_add_to_cart_form.rb b/app/overrides/add_distributor_to_add_to_cart_form.rb new file mode 100644 index 0000000000..0b0bd07ac6 --- /dev/null +++ b/app/overrides/add_distributor_to_add_to_cart_form.rb @@ -0,0 +1,4 @@ +Deface::Override.new(:virtual_path => "spree/products/_cart_form", + :replace => "[data-hook='product_price'] .add-to-cart", + :partial => "spree/products/add_to_cart", + :name => "product_add_to_cart") diff --git a/app/views/spree/products/_add_to_cart.html.haml b/app/views/spree/products/_add_to_cart.html.haml new file mode 100644 index 0000000000..3c6ef381a8 --- /dev/null +++ b/app/views/spree/products/_add_to_cart.html.haml @@ -0,0 +1,12 @@ +.add-to-cart + - if @product.has_stock? || Spree::Config[:allow_backorders] + %p Quantity + = number_field_tag (@product.has_variants? ? :quantity : "variants[#{@product.master.id}]"), 1, :class => 'title', :in => 1..@product.on_hand + %p Distributor + = select_tag "distributor_id", options_from_collection_for_select(@distributors, "id", "name", current_distributor.andand.id) + %br/ + = button_tag :class => 'large primary', :id => 'add-to-cart-button', :type => :submit do + = t(:add_to_cart) + + - else + = content_tag('strong', t(:out_of_stock)) diff --git a/spec/requests/consumer/distributors_spec.rb b/spec/requests/consumer/distributors_spec.rb index c1b3747ba8..9c58d03f84 100644 --- a/spec/requests/consumer/distributors_spec.rb +++ b/spec/requests/consumer/distributors_spec.rb @@ -78,28 +78,21 @@ feature %q{ end context "viewing a product, it provides a choice of distributor when adding to cart" do - it "displays the local distributor as the default choice when available for the current product" - it "functions with remote distributors" + it "displays the local distributor as the default choice when available for the current product" do + # Given a distributor and a product under it + distributor = create(:distributor) + product = create(:product, :distributors => [distributor]) + + # When we select the distributor and view the product + visit spree.root_path + click_link distributor.name + visit spree.product_path(product) + + # Then we should see our distributor as the default option when adding the item to our cart + page.should have_selector "select#distributor_id option[value='#{distributor.id}'][selected='selected']" + end + + it "functions with remote distributors also" end end - - - - # scenario "browsing products by distributor" do - # # Given a product at each of two distributors - # d1 = create(:distributor) - # d2 = create(:distributor) - # p1 = create(:product, :distributors => [d1]) - # p2 = create(:product, :distributors => [d2]) - - # # When I go to the home page, I should see both products - # visit spree.root_path - # page.should have_content p1.name - # page.should have_content p2.name - - # # When I filter by one distributor, I should see only the product from that distributor - # click_link d1.name - # page.should have_content p1.name - # page.should_not have_content p2.name - # end end From b2f5e23fa4fac2ece9b2f2bc8da5210f169d6060 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Sun, 24 Jun 2012 11:41:10 +1000 Subject: [PATCH 08/25] Test selecting distributor when adding item to cart, for no/remote distributor selected --- .../spree/products_controller_decorator.rb | 7 ----- .../spree/products/_add_to_cart.html.haml | 2 +- spec/requests/consumer/distributors_spec.rb | 31 ++++++++++++++++++- 3 files changed, 31 insertions(+), 9 deletions(-) diff --git a/app/controllers/spree/products_controller_decorator.rb b/app/controllers/spree/products_controller_decorator.rb index c25ca0f02a..e7964028e9 100644 --- a/app/controllers/spree/products_controller_decorator.rb +++ b/app/controllers/spree/products_controller_decorator.rb @@ -4,15 +4,8 @@ Spree::ProductsController.class_eval do include Spree::DistributorsHelper include OpenFoodWeb::SplitProductsByDistributor - before_filter :load_distributors, :only => :show - respond_override :index => { :html => { :success => lambda { @products, @products_local, @products_remote = split_products_by_distributor @products, current_distributor } } } - - def load_distributors - @distributors = Spree::Distributor.by_name - end - end diff --git a/app/views/spree/products/_add_to_cart.html.haml b/app/views/spree/products/_add_to_cart.html.haml index 3c6ef381a8..bfe44578ac 100644 --- a/app/views/spree/products/_add_to_cart.html.haml +++ b/app/views/spree/products/_add_to_cart.html.haml @@ -3,7 +3,7 @@ %p Quantity = number_field_tag (@product.has_variants? ? :quantity : "variants[#{@product.master.id}]"), 1, :class => 'title', :in => 1..@product.on_hand %p Distributor - = select_tag "distributor_id", options_from_collection_for_select(@distributors, "id", "name", current_distributor.andand.id) + = select_tag "distributor_id", options_from_collection_for_select(@product.distributors, "id", "name", current_distributor.andand.id) %br/ = button_tag :class => 'large primary', :id => 'add-to-cart-button', :type => :submit do = t(:add_to_cart) diff --git a/spec/requests/consumer/distributors_spec.rb b/spec/requests/consumer/distributors_spec.rb index 9c58d03f84..69942f4e07 100644 --- a/spec/requests/consumer/distributors_spec.rb +++ b/spec/requests/consumer/distributors_spec.rb @@ -78,6 +78,19 @@ feature %q{ end context "viewing a product, it provides a choice of distributor when adding to cart" do + it "works when no distributor is chosen" do + # Given a distributor and a product under it + distributor = create(:distributor) + product = create(:product, :distributors => [distributor]) + + # When we view the product + visit spree.product_path(product) + + # Then we should see a choice of distributor, with no default + page.should have_selector "select#distributor_id option", :text => distributor.name + page.should_not have_selector "select#distributor_id option[selected='selected']" + end + it "displays the local distributor as the default choice when available for the current product" do # Given a distributor and a product under it distributor = create(:distributor) @@ -92,7 +105,23 @@ feature %q{ page.should have_selector "select#distributor_id option[value='#{distributor.id}'][selected='selected']" end - it "functions with remote distributors also" + it "works when viewing a product from a remote distributor" do + # Given two distributors and a product under one + distributor_product = create(:distributor) + distributor_no_product = create(:distributor) + product = create(:product, :distributors => [distributor_product]) + + # When we select the distributor without the product and then view the product + visit spree.root_path + click_link distributor_no_product.name + visit spree.product_path(product) + + # Then we should see a choice of distributor, + # with no default and no option for the distributor that the product does not belong to + page.should have_selector "select#distributor_id option", :text => distributor_product.name + page.should_not have_selector "select#distributor_id option", :text => distributor_no_product.name + page.should_not have_selector "select#distributor_id option[selected='selected']" + end end end end From b5082009ced68a8c3af8ff114ad3ba3e1893d068 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Sun, 24 Jun 2012 12:13:13 +1000 Subject: [PATCH 09/25] Add pending specs for adding products to cart with distributor --- spec/requests/consumer/add_to_cart_spec.rb | 34 ++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 spec/requests/consumer/add_to_cart_spec.rb diff --git a/spec/requests/consumer/add_to_cart_spec.rb b/spec/requests/consumer/add_to_cart_spec.rb new file mode 100644 index 0000000000..fe9043d9fa --- /dev/null +++ b/spec/requests/consumer/add_to_cart_spec.rb @@ -0,0 +1,34 @@ +require 'spec_helper' + +feature %q{ + As a consumer + I want to choose a distributor when adding products to my cart + So that I can avoid making an order from many different distributors +} do + include AuthenticationWorkflow + include WebHelper + + context "adding the first product to the cart" do + it "requires the user choose a distributor" do + # Tested in orders_controller_spec.rb + end + + it "sets the order's distributor" + it "sets the user's distributor when adding a product at a remote distributor" + end + + it "does not allow the user to change distributor after a product has been added to the cart" + + context "adding a subsequent product to the cart" do + it "does not allow the user to choose a distributor" do + # Instead, they see "Your distributor for this order is XYZ" + pending + end + + it "does not allow the user to add a product from another distributor" do + # No add to cart button + # They see "Please complete your order at XYZ before shopping with another distributor." + pending + end + end +end From 99d6f0baa9d83395d91a042d9cd639f577cd7248 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Sun, 24 Jun 2012 12:13:33 +1000 Subject: [PATCH 10/25] Require a distributor when adding a product to the cart --- .../spree/orders_controller_decorator.rb | 11 +++++++++++ spec/controllers/orders_controller_spec.rb | 14 ++++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 app/controllers/spree/orders_controller_decorator.rb create mode 100644 spec/controllers/orders_controller_spec.rb diff --git a/app/controllers/spree/orders_controller_decorator.rb b/app/controllers/spree/orders_controller_decorator.rb new file mode 100644 index 0000000000..bb2d2be1aa --- /dev/null +++ b/app/controllers/spree/orders_controller_decorator.rb @@ -0,0 +1,11 @@ +Spree::OrdersController.class_eval do + before_filter :populate_order_distributor, :only => :populate + + def populate_order_distributor + @distributor = Spree::Distributor.find params[:distributor_id] + if @distributor.nil? + return false + end + + end +end diff --git a/spec/controllers/orders_controller_spec.rb b/spec/controllers/orders_controller_spec.rb new file mode 100644 index 0000000000..b78ab3b034 --- /dev/null +++ b/spec/controllers/orders_controller_spec.rb @@ -0,0 +1,14 @@ +require 'spec_helper' + +describe Spree::OrdersController do + context "adding the first product to the cart" do + it "does nothing if the user does not specify a distributor" do + create(:distributor) + p = create(:product) + + expect do + spree_put :populate, :variants => {p.id => 1} + end.to change(Spree::LineItem, :count).by(0) + end + end +end From fc5795173e62f0eb3f5f6c9851ac36c1f2c9fa38 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Sun, 24 Jun 2012 12:56:47 +1000 Subject: [PATCH 11/25] Do not add the product to cart if the user specifies a distributor that the product is not available at --- .../spree/orders_controller_decorator.rb | 23 ++++++++++++++++--- spec/controllers/orders_controller_spec.rb | 12 +++++++++- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/app/controllers/spree/orders_controller_decorator.rb b/app/controllers/spree/orders_controller_decorator.rb index bb2d2be1aa..d9cdc8bebe 100644 --- a/app/controllers/spree/orders_controller_decorator.rb +++ b/app/controllers/spree/orders_controller_decorator.rb @@ -3,9 +3,26 @@ Spree::OrdersController.class_eval do def populate_order_distributor @distributor = Spree::Distributor.find params[:distributor_id] - if @distributor.nil? - return false - end + redirect_to cart_path unless populate_valid? @distributor + end + + private + def populate_valid? distributor + # -- Distributor must be specified + return false if distributor.nil? + + # -- All products must be available under that distributor + params[:products].each do |product_id, variant_id| + product = Spree::Product.find product_id + return false unless product.distributors.include? distributor + end if params[:products] + + params[:variants].each do |variant_id, quantity| + variant = Spree::Variant.find variant_id + return false unless variant.product.distributors.include? distributor + end if params[:variants] + + true end end diff --git a/spec/controllers/orders_controller_spec.rb b/spec/controllers/orders_controller_spec.rb index b78ab3b034..9d500062a4 100644 --- a/spec/controllers/orders_controller_spec.rb +++ b/spec/controllers/orders_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Spree::OrdersController do context "adding the first product to the cart" do - it "does nothing if the user does not specify a distributor" do + it "does not add the product if the user does not specify a distributor" do create(:distributor) p = create(:product) @@ -10,5 +10,15 @@ describe Spree::OrdersController do spree_put :populate, :variants => {p.id => 1} end.to change(Spree::LineItem, :count).by(0) end + + it "does not add the product if the user specifies a distributor that the product is not available at" do + distributor_product = create(:distributor) + distributor_no_product = create(:distributor) + p = create(:product, :distributors => [distributor_product]) + + expect do + spree_put :populate, :variants => {p.id => 1}, :distributor_id => distributor_no_product.id + end.to change(Spree::LineItem, :count).by(0) + end end end From 321900642386d517fd5c8b1393d8b794b6a5b49a Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Sun, 24 Jun 2012 13:21:36 +1000 Subject: [PATCH 12/25] Push several request specs down to controller level, replace with scenario. Set order's distributor when adding product to cart --- .../spree/orders_controller_decorator.rb | 9 ++++++- spec/controllers/orders_controller_spec.rb | 16 ++++++++++++ spec/requests/consumer/add_to_cart_spec.rb | 26 ++++++++++++++----- 3 files changed, 44 insertions(+), 7 deletions(-) diff --git a/app/controllers/spree/orders_controller_decorator.rb b/app/controllers/spree/orders_controller_decorator.rb index d9cdc8bebe..b87beb7e2c 100644 --- a/app/controllers/spree/orders_controller_decorator.rb +++ b/app/controllers/spree/orders_controller_decorator.rb @@ -4,7 +4,14 @@ Spree::OrdersController.class_eval do def populate_order_distributor @distributor = Spree::Distributor.find params[:distributor_id] - redirect_to cart_path unless populate_valid? @distributor + if populate_valid? @distributor + order = current_order(true) + order.distributor = @distributor + order.save! + + else + redirect_to cart_path + end end private diff --git a/spec/controllers/orders_controller_spec.rb b/spec/controllers/orders_controller_spec.rb index 9d500062a4..1bac3596dc 100644 --- a/spec/controllers/orders_controller_spec.rb +++ b/spec/controllers/orders_controller_spec.rb @@ -1,6 +1,9 @@ require 'spec_helper' +require 'spree/core/current_order' describe Spree::OrdersController do + include Spree::Core::CurrentOrder + context "adding the first product to the cart" do it "does not add the product if the user does not specify a distributor" do create(:distributor) @@ -20,5 +23,18 @@ describe Spree::OrdersController do spree_put :populate, :variants => {p.id => 1}, :distributor_id => distributor_no_product.id end.to change(Spree::LineItem, :count).by(0) end + + it "sets the order's distributor" do + # Given a product in a distributor + d = create(:distributor) + p = create(:product, :distributors => [d]) + + # When we add the product to our cart + spree_put :populate, :variants => {p.id => 1}, :distributor_id => d.id + + # Then our order should have its distributor set to the chosen distributor + current_order(false).distributor.should == d + end + end end diff --git a/spec/requests/consumer/add_to_cart_spec.rb b/spec/requests/consumer/add_to_cart_spec.rb index fe9043d9fa..9ac434ac9b 100644 --- a/spec/requests/consumer/add_to_cart_spec.rb +++ b/spec/requests/consumer/add_to_cart_spec.rb @@ -8,13 +8,27 @@ feature %q{ include AuthenticationWorkflow include WebHelper - context "adding the first product to the cart" do - it "requires the user choose a distributor" do - # Tested in orders_controller_spec.rb - end + scenario "adding the first product to the cart" do + # Given a product and some distributors + d1 = create(:distributor) + d2 = create(:distributor) + p = create(:product, :distributors => [d1]) - it "sets the order's distributor" - it "sets the user's distributor when adding a product at a remote distributor" + # 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 + visit spree.product_path p + select d1.name, :from => 'distributor_id' + click_button 'Add To Cart' + + # Then the item should be in my cart + order = Spree::Order.last + order.line_items.first.product.should == p + + # And my order should have its distributor set to the chosen distributor + order.distributor.should == d1 end it "does not allow the user to change distributor after a product has been added to the cart" From 056733a4266101a91f072fd95b3b9b1e43bf2297 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Sun, 24 Jun 2012 13:48:55 +1000 Subject: [PATCH 13/25] Add controller spec for deselecting distributors --- .../distributors_controller_spec.rb | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/spec/controllers/distributors_controller_spec.rb b/spec/controllers/distributors_controller_spec.rb index 68ecef24d0..17a2abab83 100644 --- a/spec/controllers/distributors_controller_spec.rb +++ b/spec/controllers/distributors_controller_spec.rb @@ -4,13 +4,32 @@ require 'spree/core/current_order' describe Spree::DistributorsController do include Spree::Core::CurrentOrder + before do + stub!(:before_save_new_order) + stub!(:after_save_new_order) + end + + it "selects distributors" do d = create(:distributor) spree_get :select, :id => d.id + response.should be_redirect order = current_order(false) order.distributor.should == d + end + + it "deselects distributors" do + d = create(:distributor) + order = current_order(true) + order.distributor = d + order.save! + + spree_get :deselect response.should be_redirect + + order.reload + order.distributor.should be_nil end end From d3c80e99fc807cc8708444e4c6e610c43e18021e Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Sun, 24 Jun 2012 14:54:48 +1000 Subject: [PATCH 14/25] Do not allow changing distributor after product added to cart at controller level --- .../spree/distributors_controller.rb | 7 ++++-- .../distributors_controller_spec.rb | 22 +++++++++++++++++++ spec/requests/consumer/add_to_cart_spec.rb | 17 +++++++++++++- 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/app/controllers/spree/distributors_controller.rb b/app/controllers/spree/distributors_controller.rb index f0e195b88b..bf1deff485 100644 --- a/app/controllers/spree/distributors_controller.rb +++ b/app/controllers/spree/distributors_controller.rb @@ -13,8 +13,11 @@ module Spree distributor = Distributor.find params[:id] order = current_order(true) - order.distributor = distributor - order.save! + + if order.line_items.empty? + order.distributor = distributor + order.save! + end redirect_back_or_default(root_path) end diff --git a/spec/controllers/distributors_controller_spec.rb b/spec/controllers/distributors_controller_spec.rb index 17a2abab83..843c7e9f43 100644 --- a/spec/controllers/distributors_controller_spec.rb +++ b/spec/controllers/distributors_controller_spec.rb @@ -32,4 +32,26 @@ describe Spree::DistributorsController do order.reload order.distributor.should be_nil end + + context "when a product has been added to the cart" do + it "does not allow selecting another distributor" do + # Given some distributors and an order with a product + d1 = create(:distributor) + d2 = create(:distributor) + p = create(:product, :distributors => [d1]) + o = current_order(true) + o.add_variant(p.master, 1) + o.distributor = d1 + o.save! + + # When I attempt to select a distributor + spree_get :select, :id => d2.id + + # Then my distributor should remain unchanged + o.reload + o.distributor.should == d1 + end + + it "does not allow deselecting distributors" + end end diff --git a/spec/requests/consumer/add_to_cart_spec.rb b/spec/requests/consumer/add_to_cart_spec.rb index 9ac434ac9b..2155ba45d5 100644 --- a/spec/requests/consumer/add_to_cart_spec.rb +++ b/spec/requests/consumer/add_to_cart_spec.rb @@ -31,7 +31,22 @@ feature %q{ order.distributor.should == d1 end - it "does not allow the user to change distributor after a product has been added to the cart" + it "does not allow the user to change distributor after a product has been added to the cart" do + # Given a product and some distributors + d1 = create(:distributor) + d2 = create(:distributor) + p = create(:product, :distributors => [d1]) + + # When I add a product to my cart (which sets my distributor) + visit spree.product_path p + click_button 'Add To Cart' + page.should have_content "You are shopping at #{d1.name}" + + # Then I should not be able to change distributor + visit spree.root_path + page.should_not have_selector 'a', :text => d1.name + page.should_not have_selector 'a', :text => d2.name + end context "adding a subsequent product to the cart" do it "does not allow the user to choose a distributor" do From 7b92fcb6146005de1af018e351a99bfcd924c520 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Sun, 24 Jun 2012 14:56:27 +1000 Subject: [PATCH 15/25] Do not allow deselecting distributor after product added to cart at controller level --- .../spree/distributors_controller.rb | 7 +++++-- .../controllers/distributors_controller_spec.rb | 17 ++++++++++++++++- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/app/controllers/spree/distributors_controller.rb b/app/controllers/spree/distributors_controller.rb index bf1deff485..ce35922de7 100644 --- a/app/controllers/spree/distributors_controller.rb +++ b/app/controllers/spree/distributors_controller.rb @@ -24,8 +24,11 @@ module Spree def deselect order = current_order(true) - order.distributor = nil - order.save! + + if order.line_items.empty? + order.distributor = nil + order.save! + end redirect_back_or_default(root_path) end diff --git a/spec/controllers/distributors_controller_spec.rb b/spec/controllers/distributors_controller_spec.rb index 843c7e9f43..7d97a53860 100644 --- a/spec/controllers/distributors_controller_spec.rb +++ b/spec/controllers/distributors_controller_spec.rb @@ -52,6 +52,21 @@ describe Spree::DistributorsController do o.distributor.should == d1 end - it "does not allow deselecting distributors" + it "does not allow deselecting distributors" do + # Given a distributor and an order with a product + d = create(:distributor) + p = create(:product, :distributors => [d]) + o = current_order(true) + o.add_variant(p.master, 1) + o.distributor = d + o.save! + + # When I attempt to deselect the distributor + spree_get :deselect + + # Then my distributor should remain unchanged + o.reload + o.distributor.should == d + end end end From a41c5d473510b21b635537bccca45db56e2162f1 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Sun, 24 Jun 2012 15:15:53 +1000 Subject: [PATCH 16/25] Move distributor permissions onto order class, do not show links to change distributor if the action is invalid --- app/controllers/spree/distributors_controller.rb | 4 ++-- app/models/spree/order_decorator.rb | 5 +++++ app/views/spree/products/_source_sidebar.html.haml | 7 +++++-- spec/models/order_spec.rb | 11 +++++++++++ spec/requests/consumer/add_to_cart_spec.rb | 1 + 5 files changed, 24 insertions(+), 4 deletions(-) create mode 100644 spec/models/order_spec.rb diff --git a/app/controllers/spree/distributors_controller.rb b/app/controllers/spree/distributors_controller.rb index ce35922de7..bee2dc444d 100644 --- a/app/controllers/spree/distributors_controller.rb +++ b/app/controllers/spree/distributors_controller.rb @@ -14,7 +14,7 @@ module Spree order = current_order(true) - if order.line_items.empty? + if order.can_change_distributor? order.distributor = distributor order.save! end @@ -25,7 +25,7 @@ module Spree def deselect order = current_order(true) - if order.line_items.empty? + if order.can_change_distributor? order.distributor = nil order.save! end diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index 8818fe5465..6273a32faa 100644 --- a/app/models/spree/order_decorator.rb +++ b/app/models/spree/order_decorator.rb @@ -1,6 +1,11 @@ Spree::Order.class_eval do belongs_to :distributor + def can_change_distributor? + # Distributor may not be changed once an item has been added to the cart/order + line_items.empty? + end + # before_validation :shipping_address_from_distributor private diff --git a/app/views/spree/products/_source_sidebar.html.haml b/app/views/spree/products/_source_sidebar.html.haml index ff8e82c9ab..dc82c9ce0d 100644 --- a/app/views/spree/products/_source_sidebar.html.haml +++ b/app/views/spree/products/_source_sidebar.html.haml @@ -6,7 +6,10 @@ %h6.filter_name Shop by Distributor %ul.filter_choices + - order = current_order(false) - @distributors.each do |distributor| - %li.nowrap= link_to distributor.name, select_distributor_path(distributor) - - if current_distributor + %li.nowrap + - if order.nil? || order.can_change_distributor? + = link_to distributor.name, select_distributor_path(distributor) + - if current_distributor && order.can_change_distributor? %li.nowrap= link_to 'Leave distributor', deselect_distributors_path diff --git a/spec/models/order_spec.rb b/spec/models/order_spec.rb new file mode 100644 index 0000000000..fe27b0b887 --- /dev/null +++ b/spec/models/order_spec.rb @@ -0,0 +1,11 @@ +require 'spec_helper' + +describe Spree::Order do + it "provides permissions for changing distributor" do + p = build(:product) + + subject.can_change_distributor?.should be_true + subject.add_variant(p.master, 1) + subject.can_change_distributor?.should be_false + end +end diff --git a/spec/requests/consumer/add_to_cart_spec.rb b/spec/requests/consumer/add_to_cart_spec.rb index 2155ba45d5..98fd5f8323 100644 --- a/spec/requests/consumer/add_to_cart_spec.rb +++ b/spec/requests/consumer/add_to_cart_spec.rb @@ -46,6 +46,7 @@ feature %q{ visit spree.root_path page.should_not have_selector 'a', :text => d1.name page.should_not have_selector 'a', :text => d2.name + page.should_not have_selector 'a', :text => 'Leave distributor' end context "adding a subsequent product to the cart" do From 24ad4f53fcd55720cae823b72695938eb1fd197a Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Sun, 24 Jun 2012 19:23:44 +1000 Subject: [PATCH 17/25] Raise error if distributor changed illegally --- app/models/spree/order_decorator.rb | 5 +++++ app/views/spree/products/_source_sidebar.html.haml | 2 ++ spec/controllers/distributors_controller_spec.rb | 4 ++-- spec/models/order_spec.rb | 12 +++++++++++- 4 files changed, 20 insertions(+), 3 deletions(-) diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index 6273a32faa..d2a982438f 100644 --- a/app/models/spree/order_decorator.rb +++ b/app/models/spree/order_decorator.rb @@ -6,6 +6,11 @@ Spree::Order.class_eval do line_items.empty? end + def distributor=(distributor) + raise "You cannot change the distributor of an order with products" unless can_change_distributor? + super(distributor) + end + # before_validation :shipping_address_from_distributor private diff --git a/app/views/spree/products/_source_sidebar.html.haml b/app/views/spree/products/_source_sidebar.html.haml index dc82c9ce0d..8cf876542d 100644 --- a/app/views/spree/products/_source_sidebar.html.haml +++ b/app/views/spree/products/_source_sidebar.html.haml @@ -11,5 +11,7 @@ %li.nowrap - if order.nil? || order.can_change_distributor? = link_to distributor.name, select_distributor_path(distributor) + - else + = distributor.name - if current_distributor && order.can_change_distributor? %li.nowrap= link_to 'Leave distributor', deselect_distributors_path diff --git a/spec/controllers/distributors_controller_spec.rb b/spec/controllers/distributors_controller_spec.rb index 7d97a53860..404090836f 100644 --- a/spec/controllers/distributors_controller_spec.rb +++ b/spec/controllers/distributors_controller_spec.rb @@ -40,8 +40,8 @@ describe Spree::DistributorsController do d2 = create(:distributor) p = create(:product, :distributors => [d1]) o = current_order(true) - o.add_variant(p.master, 1) o.distributor = d1 + o.add_variant(p.master, 1) o.save! # When I attempt to select a distributor @@ -57,8 +57,8 @@ describe Spree::DistributorsController do d = create(:distributor) p = create(:product, :distributors => [d]) o = current_order(true) - o.add_variant(p.master, 1) o.distributor = d + o.add_variant(p.master, 1) o.save! # When I attempt to deselect the distributor diff --git a/spec/models/order_spec.rb b/spec/models/order_spec.rb index fe27b0b887..872d1d949f 100644 --- a/spec/models/order_spec.rb +++ b/spec/models/order_spec.rb @@ -1,11 +1,21 @@ require 'spec_helper' describe Spree::Order do - it "provides permissions for changing distributor" do + it "reveals permission for changing distributor" do p = build(:product) subject.can_change_distributor?.should be_true subject.add_variant(p.master, 1) subject.can_change_distributor?.should be_false end + + it "raises an exception if distributor is changed without permission" do + p = build(:product) + subject.add_variant(p.master, 1) + subject.can_change_distributor?.should be_false + + expect do + subject.distributor = nil + end.to raise_error "You cannot change the distributor of an order with products" + end end From 4481ca83f9126bb2467323972fc70018c36940dd Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Sun, 24 Jun 2012 19:58:09 +1000 Subject: [PATCH 18/25] OrdersController does not permit adding products with invalid distributors --- .../spree/orders_controller_decorator.rb | 6 +++ spec/controllers/orders_controller_spec.rb | 40 +++++++++++++++++++ spec/requests/consumer/add_to_cart_spec.rb | 14 ++++++- 3 files changed, 58 insertions(+), 2 deletions(-) diff --git a/app/controllers/spree/orders_controller_decorator.rb b/app/controllers/spree/orders_controller_decorator.rb index b87beb7e2c..be6b716639 100644 --- a/app/controllers/spree/orders_controller_decorator.rb +++ b/app/controllers/spree/orders_controller_decorator.rb @@ -30,6 +30,12 @@ Spree::OrdersController.class_eval do return false unless variant.product.distributors.include? distributor end if params[:variants] + # -- If products in cart, distributor can't be changed + order = current_order(false) + if !order.nil? && order.distributor != distributor + return false + end + true end end diff --git a/spec/controllers/orders_controller_spec.rb b/spec/controllers/orders_controller_spec.rb index 1bac3596dc..d7b82ae613 100644 --- a/spec/controllers/orders_controller_spec.rb +++ b/spec/controllers/orders_controller_spec.rb @@ -35,6 +35,46 @@ describe Spree::OrdersController do # Then our order should have its distributor set to the chosen distributor current_order(false).distributor.should == d end + end + context "adding a subsequent product to the cart" do + before(:each) do + # Given a product and a distributor + @distributor = create(:distributor) + @product = create(:product, :distributors => [@distributor]) + + # And the product is in the cart + spree_put :populate, :variants => {@product.id => 1}, :distributor_id => @distributor.id + current_order(false).line_items.map { |li| li.product }.should == [@product] + current_order(false).distributor.should == @distributor + end + + it "does not add the product if the product is not available at the order's distributor" do + # Given a product at another distributor + d2 = create(:distributor) + p2 = create(:product, :distributors => [d2]) + + # When I attempt to add the product to the cart + spree_put :populate, :variants => {p2.id => 1}, :distributor_id => d2.id + + # Then the product should not be added to the cart + current_order(false).line_items.map { |li| li.product }.should == [@product] + current_order(false).distributor.should == @distributor + end + + it "does not add the product if the product is not available at the given distributor" do + # Given a product at another distributor + d2 = create(:distributor) + p2 = create(:product, :distributors => [d2]) + + # When I attempt to add the product to the cart with a fake distributor_id + spree_put :populate, :variants => {p2.id => 1}, :distributor_id => @distributor.id + + # Then the product should not be added to the cart + current_order(false).line_items.map { |li| li.product }.should == [@product] + current_order(false).distributor.should == @distributor + end + + it "does not add the product if the chosen distributor is different from the order's distributor" end end diff --git a/spec/requests/consumer/add_to_cart_spec.rb b/spec/requests/consumer/add_to_cart_spec.rb index 98fd5f8323..fbe51c5e7a 100644 --- a/spec/requests/consumer/add_to_cart_spec.rb +++ b/spec/requests/consumer/add_to_cart_spec.rb @@ -51,8 +51,18 @@ feature %q{ context "adding a subsequent product to the cart" do it "does not allow the user to choose a distributor" do - # Instead, they see "Your distributor for this order is XYZ" - pending + # Given a product under a distributor + d = create(:distributor) + p = create(:product, :distributors => [d]) + + # And a product in my cart + visit spree.product_path p + click_button 'Add To Cart' + + # When I go to add it again, I should not have a choice of distributor + visit spree.product_path p + page.should_not have_selector 'select#distributor_id' + page.should have_selector 'p', :text => "Your distributor for this order is #{d.name}" end it "does not allow the user to add a product from another distributor" do From df831a912873febd1624a01d2fe6cae203c91ea5 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Sun, 24 Jun 2012 20:14:07 +1000 Subject: [PATCH 19/25] Do not add the product to cart if the chosen distributor is different from the order's distributor --- .../spree/orders_controller_decorator.rb | 2 +- spec/controllers/orders_controller_spec.rb | 25 +++++++++++++------ 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/app/controllers/spree/orders_controller_decorator.rb b/app/controllers/spree/orders_controller_decorator.rb index be6b716639..d442380f2d 100644 --- a/app/controllers/spree/orders_controller_decorator.rb +++ b/app/controllers/spree/orders_controller_decorator.rb @@ -2,7 +2,7 @@ Spree::OrdersController.class_eval do before_filter :populate_order_distributor, :only => :populate def populate_order_distributor - @distributor = Spree::Distributor.find params[:distributor_id] + @distributor = params.key?(:distributor_id) ? Spree::Distributor.find(params[:distributor_id]) : nil if populate_valid? @distributor order = current_order(true) diff --git a/spec/controllers/orders_controller_spec.rb b/spec/controllers/orders_controller_spec.rb index d7b82ae613..89f4ef46ba 100644 --- a/spec/controllers/orders_controller_spec.rb +++ b/spec/controllers/orders_controller_spec.rb @@ -45,8 +45,8 @@ describe Spree::OrdersController do # And the product is in the cart spree_put :populate, :variants => {@product.id => 1}, :distributor_id => @distributor.id - current_order(false).line_items.map { |li| li.product }.should == [@product] - current_order(false).distributor.should == @distributor + current_order(false).line_items.reload.map { |li| li.product }.should == [@product] + current_order(false).distributor.reload.should == @distributor end it "does not add the product if the product is not available at the order's distributor" do @@ -58,8 +58,8 @@ describe Spree::OrdersController do spree_put :populate, :variants => {p2.id => 1}, :distributor_id => d2.id # Then the product should not be added to the cart - current_order(false).line_items.map { |li| li.product }.should == [@product] - current_order(false).distributor.should == @distributor + current_order(false).line_items.reload.map { |li| li.product }.should == [@product] + current_order(false).distributor.reload.should == @distributor end it "does not add the product if the product is not available at the given distributor" do @@ -71,10 +71,21 @@ describe Spree::OrdersController do spree_put :populate, :variants => {p2.id => 1}, :distributor_id => @distributor.id # Then the product should not be added to the cart - current_order(false).line_items.map { |li| li.product }.should == [@product] - current_order(false).distributor.should == @distributor + current_order(false).line_items.reload.map { |li| li.product }.should == [@product] + current_order(false).distributor.reload.should == @distributor end - it "does not add the product if the chosen distributor is different from the order's distributor" + it "does not add the product if the chosen distributor is different from the order's distributor" do + # Given a product that's available at the chosen distributor and another distributor + d2 = create(:distributor) + p2 = create(:product, :distributors => [@distributor, d2]) + + # When I attempt to add the product to the cart with the alternate distributor + spree_put :populate, :variants => {p2.id => 1}, :distributor_id => d2 + + # Then the product should not be added to the cart + current_order(false).line_items.reload.map { |li| li.product }.should == [@product] + current_order(false).distributor.reload.should == @distributor + end end end From 98bcc9ce71bbc54603f257c54226cf0adba946f8 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 25 Jun 2012 06:57:24 +1000 Subject: [PATCH 20/25] When adding first product to cart, add the product and set the distributor even if the order has a different distributor set --- .../spree/orders_controller_decorator.rb | 2 +- spec/controllers/orders_controller_spec.rb | 21 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/app/controllers/spree/orders_controller_decorator.rb b/app/controllers/spree/orders_controller_decorator.rb index d442380f2d..eb9ed95335 100644 --- a/app/controllers/spree/orders_controller_decorator.rb +++ b/app/controllers/spree/orders_controller_decorator.rb @@ -32,7 +32,7 @@ Spree::OrdersController.class_eval do # -- If products in cart, distributor can't be changed order = current_order(false) - if !order.nil? && order.distributor != distributor + if !order.nil? && !order.can_change_distributor? && order.distributor != distributor return false end diff --git a/spec/controllers/orders_controller_spec.rb b/spec/controllers/orders_controller_spec.rb index 89f4ef46ba..6204cbcc60 100644 --- a/spec/controllers/orders_controller_spec.rb +++ b/spec/controllers/orders_controller_spec.rb @@ -4,6 +4,11 @@ require 'spree/core/current_order' describe Spree::OrdersController do include Spree::Core::CurrentOrder + def current_user + controller.current_user + end + + context "adding the first product to the cart" do it "does not add the product if the user does not specify a distributor" do create(:distributor) @@ -24,6 +29,22 @@ describe Spree::OrdersController do end.to change(Spree::LineItem, :count).by(0) end + it "adds the product and sets the distributor even if the order has a different distributor set" do + distributor_product = create(:distributor) + distributor_no_product = create(:distributor) + p = create(:product, :distributors => [distributor_product]) + + order = current_order(true) + order.distributor = distributor_no_product + order.save! + + expect do + spree_put :populate, :variants => {p.id => 1}, :distributor_id => distributor_product.id + end.to change(Spree::LineItem, :count).by(1) + + order.reload.distributor.should == distributor_product + end + it "sets the order's distributor" do # Given a product in a distributor d = create(:distributor) From 178f581452d733218b04695eacc1b977ebd868ba Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 25 Jun 2012 07:25:31 +1000 Subject: [PATCH 21/25] When an item has been added to the cart, do not allow the user to select a distributor when adding a subsequent item --- app/views/spree/products/_add_to_cart.html.haml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/app/views/spree/products/_add_to_cart.html.haml b/app/views/spree/products/_add_to_cart.html.haml index bfe44578ac..dd01aa17db 100644 --- a/app/views/spree/products/_add_to_cart.html.haml +++ b/app/views/spree/products/_add_to_cart.html.haml @@ -2,8 +2,12 @@ - if @product.has_stock? || Spree::Config[:allow_backorders] %p Quantity = number_field_tag (@product.has_variants? ? :quantity : "variants[#{@product.master.id}]"), 1, :class => 'title', :in => 1..@product.on_hand - %p Distributor - = select_tag "distributor_id", options_from_collection_for_select(@product.distributors, "id", "name", current_distributor.andand.id) + - order = current_order(false) + - if order.nil? || order.can_change_distributor? + %p Distributor + = select_tag "distributor_id", options_from_collection_for_select(@product.distributors, "id", "name", current_distributor.andand.id) + - else + %p= "Your distributor for this order is #{order.distributor.name}" %br/ = button_tag :class => 'large primary', :id => 'add-to-cart-button', :type => :submit do = t(:add_to_cart) From f4108921ed315ee1a45866059f38f5b9d3196966 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 25 Jun 2012 08:45:08 +1000 Subject: [PATCH 22/25] When adding subsequent product to cart, do not allow the user to add a product from another distributor --- app/models/spree/order_decorator.rb | 6 +++++ .../spree/products/_add_to_cart.html.haml | 10 ++++--- spec/models/order_spec.rb | 26 +++++++++++++++++++ spec/requests/consumer/add_to_cart_spec.rb | 19 +++++++++++--- 4 files changed, 55 insertions(+), 6 deletions(-) diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index d2a982438f..bbe933fb58 100644 --- a/app/models/spree/order_decorator.rb +++ b/app/models/spree/order_decorator.rb @@ -11,6 +11,12 @@ Spree::Order.class_eval do super(distributor) end + def can_add_product_to_cart?(product) + can_change_distributor? || product.distributors.include?(distributor) + end + + + # before_validation :shipping_address_from_distributor private diff --git a/app/views/spree/products/_add_to_cart.html.haml b/app/views/spree/products/_add_to_cart.html.haml index dd01aa17db..1877cb5215 100644 --- a/app/views/spree/products/_add_to_cart.html.haml +++ b/app/views/spree/products/_add_to_cart.html.haml @@ -1,5 +1,11 @@ .add-to-cart - - if @product.has_stock? || Spree::Config[:allow_backorders] + - if !@product.has_stock? && !Spree::Config[:allow_backorders] + = content_tag('strong', t(:out_of_stock)) + + - elsif current_order(false) && !current_order(false).can_add_product_to_cart?(@product) + %p= "Please complete your order at #{current_distributor.name} before shopping with another distributor." + + - else %p Quantity = number_field_tag (@product.has_variants? ? :quantity : "variants[#{@product.master.id}]"), 1, :class => 'title', :in => 1..@product.on_hand - order = current_order(false) @@ -12,5 +18,3 @@ = button_tag :class => 'large primary', :id => 'add-to-cart-button', :type => :submit do = t(:add_to_cart) - - else - = content_tag('strong', t(:out_of_stock)) diff --git a/spec/models/order_spec.rb b/spec/models/order_spec.rb index 872d1d949f..33a04f4264 100644 --- a/spec/models/order_spec.rb +++ b/spec/models/order_spec.rb @@ -18,4 +18,30 @@ describe Spree::Order do subject.distributor = nil end.to raise_error "You cannot change the distributor of an order with products" end + + it "reveals permission for adding products to the cart" do + d1 = create(:distributor) + d2 = create(:distributor) + + p_first = create(:product, :distributors => [d1]) + p_subsequent_same_dist = create(:product, :distributors => [d1]) + p_subsequent_other_dist = create(:product, :distributors => [d2]) + + # We need to set distributor, since order.add_variant does not, and + # we also need to save the order so that line items can be added to + # the association. + subject.distributor = d1 + subject.save! + + # The first product in the cart can be added + subject.can_add_product_to_cart?(p_first).should be_true + subject.add_variant(p_first.master, 1) + + # A subsequent product can be added if the distributor matches + subject.can_add_product_to_cart?(p_subsequent_same_dist).should be_true + subject.add_variant(p_subsequent_same_dist.master, 1) + + # And cannot be added if it does not match + subject.can_add_product_to_cart?(p_subsequent_other_dist).should be_false + end end diff --git a/spec/requests/consumer/add_to_cart_spec.rb b/spec/requests/consumer/add_to_cart_spec.rb index fbe51c5e7a..7d5be2fad5 100644 --- a/spec/requests/consumer/add_to_cart_spec.rb +++ b/spec/requests/consumer/add_to_cart_spec.rb @@ -66,9 +66,22 @@ feature %q{ end it "does not allow the user to add a product from another distributor" do - # No add to cart button - # They see "Please complete your order at XYZ before shopping with another distributor." - pending + # Given two products, each at a different distributor + d1 = create(:distributor) + d2 = create(:distributor) + p1 = create(:product, :distributors => [d1]) + p2 = create(:product, :distributors => [d2]) + + # When I add one of them to my cart + visit spree.product_path p1 + click_button 'Add To Cart' + + # And I attempt to add the other + visit spree.product_path p2 + + # Then I should not be allowed to add the product + page.should_not have_selector "button#add-to-cart-button" + page.should have_content "Please complete your order at #{d1.name} before shopping with another distributor." end end end From a7df5a2f1e2438ad90d0029593e6e0562ba31954 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 25 Jun 2012 08:57:53 +1000 Subject: [PATCH 23/25] Include jquery, add basic styling for add to cart form, split supplier and distributors in product details sidebar --- app/assets/javascripts/store/all.js | 1 + app/assets/stylesheets/store/openfoodweb.css.scss | 13 +++++++++++++ app/views/spree/products/_add_to_cart.html.haml | 4 ++-- app/views/spree/products/_source.html.haml | 3 +++ 4 files changed, 19 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/store/all.js b/app/assets/javascripts/store/all.js index 9c176e620b..723f03af31 100644 --- a/app/assets/javascripts/store/all.js +++ b/app/assets/javascripts/store/all.js @@ -5,6 +5,7 @@ // the compiled file. // +//= require 'jquery' //= require store/spree_core //= require store/spree_auth //= require store/spree_promo diff --git a/app/assets/stylesheets/store/openfoodweb.css.scss b/app/assets/stylesheets/store/openfoodweb.css.scss index 3b0c18b6cb..73e4ab7718 100644 --- a/app/assets/stylesheets/store/openfoodweb.css.scss +++ b/app/assets/stylesheets/store/openfoodweb.css.scss @@ -107,3 +107,16 @@ ul.product-listing { border-radius: 10px; background-color: #def; } + + +/* Add to cart form on product details page */ +#cart-form { + .error-distributor { + font-size: 120%; + font-weight: bold; + color: #f00; + } + + .distributor-fixed { + } +} diff --git a/app/views/spree/products/_add_to_cart.html.haml b/app/views/spree/products/_add_to_cart.html.haml index 1877cb5215..bdeda7c571 100644 --- a/app/views/spree/products/_add_to_cart.html.haml +++ b/app/views/spree/products/_add_to_cart.html.haml @@ -3,7 +3,7 @@ = content_tag('strong', t(:out_of_stock)) - elsif current_order(false) && !current_order(false).can_add_product_to_cart?(@product) - %p= "Please complete your order at #{current_distributor.name} before shopping with another distributor." + .error-distributor= "Please complete your order at #{current_distributor.name} before shopping with another distributor." - else %p Quantity @@ -13,7 +13,7 @@ %p Distributor = select_tag "distributor_id", options_from_collection_for_select(@product.distributors, "id", "name", current_distributor.andand.id) - else - %p= "Your distributor for this order is #{order.distributor.name}" + .distributor-fixed= "Your distributor for this order is #{order.distributor.name}" %br/ = button_tag :class => 'large primary', :id => 'add-to-cart-button', :type => :submit do = t(:add_to_cart) diff --git a/app/views/spree/products/_source.html.haml b/app/views/spree/products/_source.html.haml index 48ac00e756..dc7f033b91 100644 --- a/app/views/spree/products/_source.html.haml +++ b/app/views/spree/products/_source.html.haml @@ -7,6 +7,9 @@ %td %strong Supplier %td= @product.supplier.name + %br/ + %table#product-source.table-display{:width => "100%"} + %tbody - @product.distributors.each do |distributor| %tr.even %td From 6cd9bff29ffbdfd3c22591a53430ee94cae34967 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 25 Jun 2012 09:06:56 +1000 Subject: [PATCH 24/25] Convert most templates from erb to haml --- app/views/spree/shared/_filters.html.erb | 19 ------------- app/views/spree/shared/_filters.html.haml | 15 +++++++++++ app/views/spree/shared/_products.html.erb | 31 ---------------------- app/views/spree/shared/_products.html.haml | 23 ++++++++++++++++ 4 files changed, 38 insertions(+), 50 deletions(-) delete mode 100644 app/views/spree/shared/_filters.html.erb create mode 100644 app/views/spree/shared/_filters.html.haml delete mode 100644 app/views/spree/shared/_products.html.erb create mode 100644 app/views/spree/shared/_products.html.haml diff --git a/app/views/spree/shared/_filters.html.erb b/app/views/spree/shared/_filters.html.erb deleted file mode 100644 index b2e89c205c..0000000000 --- a/app/views/spree/shared/_filters.html.erb +++ /dev/null @@ -1,19 +0,0 @@ -<% filters = @taxon ? @taxon.applicable_filters : [] %> -<% unless filters.empty? %> - -<% end %> diff --git a/app/views/spree/shared/_filters.html.haml b/app/views/spree/shared/_filters.html.haml new file mode 100644 index 0000000000..728dbbc0ef --- /dev/null +++ b/app/views/spree/shared/_filters.html.haml @@ -0,0 +1,15 @@ +- filters = @taxon ? @taxon.applicable_filters : [] +- unless filters.empty? + %nav#filters + - params[:search] ||= {} + - filters.each do |filter| + - labels = filter[:labels] || filter[:conds].map {|m,c| [m,m]} + - next if labels.empty? + + %h6.filter_name= "Shop by #{filter[:name]}" + + %ul.filter_choices + - labels.each do |nm,val| + %li.nowrap + - active = params[:search][filter[:scope]] && params[:search][filter[:scope]].include?(val.to_s) + = link_to nm, "?search[#{filter[:scope].to_s}][]=#{CGI.escape(val)}" diff --git a/app/views/spree/shared/_products.html.erb b/app/views/spree/shared/_products.html.erb deleted file mode 100644 index 0d633e7415..0000000000 --- a/app/views/spree/shared/_products.html.erb +++ /dev/null @@ -1,31 +0,0 @@ -<% - paginated_products = @searcher.retrieve_products if params.key?(:keywords) - paginated_products ||= products -%> -<% if products.empty? %> - <%= t(:no_products_found) %> -<% elsif params.key?(:keywords) %> -
<%= t(:search_results, :keywords => h(params[:keywords])) %>
-<% end %> - -<% if products.any? %> -
    - <% reset_cycle('default') %> - <% products.each do |product| %> - <% if Spree::Config[:show_zero_stock_products] || product.has_stock? %> -
  • " data-hook="products_list_item" itemscope itemtype="http://schema.org/Product"> -
    - <%= link_to small_image(product, :itemprop => "image"), product, :itemprop => 'url' %> -
    - <%= link_to truncate(product.name, :length => 50), product, :class => 'info', :itemprop => "name", :title => product.name %> - <%= number_to_currency product.price %> -
  • - <% end %> - <% end %> -
-<% end %> - -<% if paginated_products.respond_to?(:num_pages) - params.delete(:search) - params.delete(:taxon) -%><%= paginate paginated_products %><% end %> diff --git a/app/views/spree/shared/_products.html.haml b/app/views/spree/shared/_products.html.haml new file mode 100644 index 0000000000..9b1e421009 --- /dev/null +++ b/app/views/spree/shared/_products.html.haml @@ -0,0 +1,23 @@ +- paginated_products = @searcher.retrieve_products if params.key?(:keywords) +- paginated_products ||= products + +- if products.empty? + = t(:no_products_found) +- elsif params.key?(:keywords) + %h6.search-results-title= t(:search_results, :keywords => h(params[:keywords])) + +- if products.any? + %ul.inline.product-listing{"data-hook" => ""} + - reset_cycle('default') + - products.each do |product| + - if Spree::Config[:show_zero_stock_products] || product.has_stock? + %li{:class => "columns three #{cycle("alpha", "secondary", "", "omega secondary")}", "data-hook" => "products_list_item", :id => "product_#{product.id}", :itemscope => "", :itemtype => "http://schema.org/Product"} + .product-image + = link_to small_image(product, :itemprop => "image"), product, :itemprop => 'url' + = link_to truncate(product.name, :length => 50), product, :class => 'info', :itemprop => "name", :title => product.name + %span.price.selling{:itemprop => "price"}= number_to_currency product.price + +- if paginated_products.respond_to?(:num_pages) + - params.delete(:search) + - params.delete(:taxon) + = paginate paginated_products From 32dca814671d95130866110b3ddd1ea664294d8e Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 25 Jun 2012 09:10:34 +1000 Subject: [PATCH 25/25] Update spec for markup change --- spec/requests/consumer/add_to_cart_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/requests/consumer/add_to_cart_spec.rb b/spec/requests/consumer/add_to_cart_spec.rb index 7d5be2fad5..14938dcc8b 100644 --- a/spec/requests/consumer/add_to_cart_spec.rb +++ b/spec/requests/consumer/add_to_cart_spec.rb @@ -62,7 +62,7 @@ feature %q{ # When I go to add it again, I should not have a choice of distributor visit spree.product_path p page.should_not have_selector 'select#distributor_id' - page.should have_selector 'p', :text => "Your distributor for this order is #{d.name}" + page.should have_selector '.distributor-fixed', :text => "Your distributor for this order is #{d.name}" end it "does not allow the user to add a product from another distributor" do