diff --git a/app/assets/stylesheets/store/openfoodweb.css.scss b/app/assets/stylesheets/store/openfoodweb.css.scss index 12fc37d089..fbcfd31122 100644 --- a/app/assets/stylesheets/store/openfoodweb.css.scss +++ b/app/assets/stylesheets/store/openfoodweb.css.scss @@ -50,7 +50,7 @@ nav#filters { .filter_choices { padding-left: 20px; - margin-bottom: 20px; + margin-bottom: 5px; list-style: disc outside; li { @@ -62,6 +62,10 @@ nav#filters { } } } + + input[type=submit] { + margin-bottom: 15px; + } } diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 3afedf9f8c..3a540b2570 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -10,9 +10,16 @@ class ApplicationController < ActionController::Base end def load_data_for_sidebar - @suppliers = Enterprise.is_primary_producer + sidebar_distributors_limit = false + sidebar_suppliers_limit = false + @order_cycles = OrderCycle.active - @distributors = Enterprise.active_distributors + + @sidebar_suppliers = Enterprise.is_primary_producer.with_supplied_active_products_on_hand.limit(sidebar_suppliers_limit) + @total_suppliers = Enterprise.is_primary_producer.distinct_count + + @sidebar_distributors = Enterprise.active_distributors.by_name.limit(sidebar_distributors_limit) + @total_distributors = Enterprise.is_distributor.distinct_count end # All render calls within the block will be performed with the specified format diff --git a/app/controllers/enterprises_controller.rb b/app/controllers/enterprises_controller.rb index d93db83ab0..1c314670d5 100644 --- a/app/controllers/enterprises_controller.rb +++ b/app/controllers/enterprises_controller.rb @@ -16,6 +16,9 @@ class EnterprisesController < BaseController format.js do @distributor_details = Hash[@distributors.map { |d| [d.id, render_to_string(:partial => 'enterprises/distributor_details', :locals => {:distributor => d})] }] end + format.html do + @distributors + end end end diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index 885ba0cddf..868ec7be52 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -17,7 +17,16 @@ class Enterprise < ActiveRecord::Base scope :by_name, order('name') scope :is_primary_producer, where(:is_primary_producer => true) scope :is_distributor, where(:is_distributor => true) - scope :with_distributed_active_products_on_hand, lambda { joins(:distributed_products).where('spree_products.deleted_at IS NULL AND spree_products.available_on <= ? AND spree_products.count_on_hand > 0', Time.now).select('distinct(enterprises.*)') } + scope :with_supplied_active_products_on_hand, lambda { + joins(:supplied_products) + .where('spree_products.deleted_at IS NULL AND spree_products.available_on <= ? AND spree_products.count_on_hand > 0', Time.now) + .uniq + } + scope :with_distributed_active_products_on_hand, lambda { + joins(:distributed_products) + .where('spree_products.deleted_at IS NULL AND spree_products.available_on <= ? AND spree_products.count_on_hand > 0', Time.now) + .uniq + } scope :with_distributed_products_outer, joins('LEFT OUTER JOIN product_distributions ON product_distributions.distributor_id = enterprises.id'). @@ -41,6 +50,12 @@ class Enterprise < ActiveRecord::Base select('DISTINCT enterprises.*') } + + # Force a distinct count to work around relation count issue https://github.com/rails/rails/issues/5554 + def self.distinct_count + count(distinct: true) + end + def has_supplied_products_on_hand? self.supplied_products.where('count_on_hand > 0').present? end diff --git a/app/views/enterprises/distributors.html.haml b/app/views/enterprises/distributors.html.haml new file mode 100644 index 0000000000..941b65237b --- /dev/null +++ b/app/views/enterprises/distributors.html.haml @@ -0,0 +1,12 @@ +- content_for :sidebar do + %div{'data-hook' => "homepage_sidebar_navigation"} + = render 'spree/sidebar' + + +%h1 Distributors + += cms_page_content(:content, Cms::Page.find_by_full_path('/enterprises/distributors')) + +%ul.enterprises + - @distributors.each do |distributor| + %li= link_to distributor.name, distributor diff --git a/app/views/spree/products/_source_sidebar.html.haml b/app/views/spree/products/_source_sidebar.html.haml index 9a87c353cb..3a1efc30a9 100644 --- a/app/views/spree/products/_source_sidebar.html.haml +++ b/app/views/spree/products/_source_sidebar.html.haml @@ -1,19 +1,28 @@ %nav#filters - %h6.filter_name Shop by Supplier - %ul.filter_choices - - @suppliers.each do |supplier| - - if supplier.has_supplied_products_on_hand? + %div#distributor_filter + %h6.filter_name Shop by Distributor + %ul.filter_choices + - order = current_order(false) + - validator = DistributorChangeValidator.new(order) + - @sidebar_distributors.each do |distributor| + %li.nowrap + - if order.nil? || order.distributor == distributor || validator.can_change_to_distributor?(distributor) + = link_to distributor.name, [main_app, distributor] + - else + %abbr(title="One or more of the products in your cart is not available from this distributor")= distributor.name + - if @total_distributors > @sidebar_distributors.length + - distributors_more = @total_distributors - @sidebar_distributors.length + %span.filter_more + = "#{distributors_more} more..." + = button_to 'Browse All Distributors', main_app.distributors_enterprises_path, :method => :get + + %div#supplier_filter + %h6.filter_name Shop by Supplier + %ul.filter_choices + - @sidebar_suppliers.each do |supplier| %li.nowrap= link_to supplier.name, [main_app, supplier] + - if @total_suppliers > @sidebar_suppliers.length + - suppliers_more = @total_suppliers - @sidebar_suppliers.length + %span.filter_more + = "#{suppliers_more} more..." = button_to 'Browse All Suppliers', main_app.suppliers_enterprises_path, :method => :get - - %h6.filter_name Shop by Distributor - %ul.filter_choices - - order = current_order(false) - - validator = DistributionChangeValidator.new(order) - - @distributors.each do |distributor| - %li.nowrap - - if order.nil? || order.distributor == distributor || validator.can_change_to_distributor?(distributor) - = link_to distributor.name, [main_app, distributor] - - else - %abbr(title="One or more of the products in your cart is not available from this distributor")= distributor.name - = button_to 'Browse All Distributors', deselect_distributor_orders_path, :method => :get diff --git a/spec/features/consumer/browse_products_spec.rb b/spec/features/consumer/browse_products_spec.rb index 0bb43da2b9..31cb2a6bfc 100644 --- a/spec/features/consumer/browse_products_spec.rb +++ b/spec/features/consumer/browse_products_spec.rb @@ -67,20 +67,6 @@ feature %q{ end end - it "allows the user to leave the distributor" do - # Given a distributor with a product - d = create(:distributor_enterprise, :name => 'Melb Uni Co-op') - p1 = create(:product, :distributors => [d]) - - # When I select the distributor and then leave it - visit spree.select_distributor_order_path(d) - visit spree.root_path - click_button 'Browse All Distributors' - - # Then I should have left the distributor - page.should_not have_selector '#current-distribution', :text => 'You are shopping at Melb Uni Co-op' - 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 diff --git a/spec/features/consumer/distributors_spec.rb b/spec/features/consumer/distributors_spec.rb index cb798b1ed3..2ca470ac56 100644 --- a/spec/features/consumer/distributors_spec.rb +++ b/spec/features/consumer/distributors_spec.rb @@ -8,7 +8,7 @@ feature %q{ include AuthenticationWorkflow include WebHelper - scenario "viewing a list of distributors" do + scenario "viewing a list of distributors in the sidebar" do # Given some distributors d1 = create(:distributor_enterprise) d2 = create(:distributor_enterprise) @@ -26,6 +26,59 @@ feature %q{ page.should_not have_selector 'a', :text => d3.name end + scenario "viewing a list of distributors (with active products) in the sidebar when there's some inactive distributors" do + # Given some distributors + d1 = create(:distributor_enterprise) + d2 = create(:distributor_enterprise) + d3 = create(:distributor_enterprise) + d4 = create(:distributor_enterprise) + d5 = create(:distributor_enterprise) + d6 = create(:distributor_enterprise) + + # And some of those distributors have a product + create(:product, :distributors => [d1]) + create(:product, :distributors => [d3], :on_hand => 0) + + # And no limit set for the sidebar + sidebar_distributors_limit = false + + # When I go to the home page + visit spree.root_path + + # Then I should see a list containing all the distributors that have active products in stock + page.should have_selector 'a', :text => d1.name + page.should_not have_selector 'a', :text => d2.name #has no products + page.should_not have_selector 'a', :text => d3.name #has no products on hand + + # And I should see '5 more' + distributors_more = Enterprise.is_distributor.distinct_count - Enterprise.is_distributor.with_distributed_active_products_on_hand.by_name.limit(sidebar_distributors_limit).length + page.should have_selector '#distributor_filter span.filter_more', :text => "#{distributors_more} more" + + # And I should (always) see a browse distributors button + page.should have_selector "#distributor_filter input[value='Browse All Distributors']" + end + + scenario "viewing a list of all distributors" do + # Given some distributors + d1 = create(:distributor_enterprise) + d2 = create(:distributor_enterprise) + d3 = create(:distributor_enterprise) + + # And some of those distributors have a product + create(:product, :distributors => [d1]) + create(:product, :distributors => [d3]) + + # When I go to the distributors listing page + visit spree.root_path + click_button 'Browse All Distributors' + + # Then I should see a list containing all the distributors + page.should have_selector '#content a', :text => d1.name + page.should have_selector '#content a', :text => d2.name + page.should have_selector '#content a', :text => d3.name + end + + scenario "viewing a distributor" do # Given some distributors with products d1 = create(:distributor_enterprise, :long_description => "

Hello, world!

") @@ -45,5 +98,4 @@ feature %q{ page.should have_content p1.name page.should_not have_content p2.name end - end diff --git a/spec/features/consumer/suppliers_spec.rb b/spec/features/consumer/suppliers_spec.rb index c52e3c6f86..2dee0f1a5f 100644 --- a/spec/features/consumer/suppliers_spec.rb +++ b/spec/features/consumer/suppliers_spec.rb @@ -8,23 +8,36 @@ feature %q{ include AuthenticationWorkflow include WebHelper - scenario "viewing a list of suppliers in the sidebar" do + scenario "viewing a list of suppliers (with active products) in the sidebar when there's 5 or fewer" do # Given some suppliers s1 = create(:supplier_enterprise) s2 = create(:supplier_enterprise) s3 = create(:supplier_enterprise) + s4 = create(:supplier_enterprise) + s5 = create(:supplier_enterprise) + s6 = create(:supplier_enterprise) # And some of those suppliers have a product create(:product, :supplier => s1) - create(:product, :supplier => s3) + create(:product, :supplier => s3, :on_hand => 0) + + # And no limit set for the sidebar + sidebar_suppliers_limit = false # When I go to the home page visit spree.root_path - # Then I should see a list containing all the suppliers that have products in stock + # Then I should see a list containing all the suppliers that have active products in stock page.should have_selector 'a', :text => s1.name - page.should have_selector 'a', :text => s3.name - page.should_not have_selector 'a', :text => s2.name + page.should_not have_selector 'a', :text => s2.name #has no products + page.should_not have_selector 'a', :text => s3.name #has no products on hand + + # And I should see '5 more' + suppliers_more = Enterprise.is_primary_producer.distinct_count - Enterprise.is_primary_producer.with_supplied_active_products_on_hand.limit(sidebar_suppliers_limit).length + page.should have_selector '#supplier_filter span.filter_more', :text => "#{suppliers_more} more" + + # And I should (always) see a browse suppliers button + page.should have_selector "#supplier_filter input[value='Browse All Suppliers']" end scenario "viewing a list of all suppliers" do diff --git a/spec/models/enterprises_spec.rb b/spec/models/enterprises_spec.rb index b4f4c153ec..1a49815ca4 100644 --- a/spec/models/enterprises_spec.rb +++ b/spec/models/enterprises_spec.rb @@ -72,6 +72,43 @@ describe Enterprise do Enterprise.with_distributed_active_products_on_hand.sort.should == [d1, d2] end + + it "returns distributors with available products in stock" do + d1 = create(:distributor_enterprise) # two products on hand + d2 = create(:distributor_enterprise) # one product on hand + d3 = create(:distributor_enterprise) # product on hand but not yet available + d4 = create(:distributor_enterprise) # no products on hand + d5 = create(:distributor_enterprise) # deleted product + d6 = create(:distributor_enterprise) # no products + create(:product, :distributors => [d1, d2], :on_hand => 5) + create(:product, :distributors => [d1], :on_hand => 5) + create(:product, :distributors => [d3], :on_hand => 5, :available_on => 1.week.from_now) + create(:product, :distributors => [d4], :on_hand => 0) + create(:product, :distributors => [d5]).delete + + Enterprise.with_distributed_active_products_on_hand.sort.should == [d1, d2] + Enterprise.with_distributed_active_products_on_hand.distinct_count.should == 2 + end + end + + describe "with_supplied_active_products_on_hand" do + it "returns suppliers with available products in stock" do + d1 = create(:supplier_enterprise) # two products on hand + d2 = create(:supplier_enterprise) # one product on hand + d3 = create(:supplier_enterprise) # product on hand but not yet available + d4 = create(:supplier_enterprise) # no products on hand + d5 = create(:supplier_enterprise) # deleted product + d6 = create(:supplier_enterprise) # no products + create(:product, :supplier => d1, :on_hand => 5) + create(:product, :supplier => d1, :on_hand => 5) + create(:product, :supplier => d2, :on_hand => 5) + create(:product, :supplier => d3, :on_hand => 5, :available_on => 1.week.from_now) + create(:product, :supplier => d4, :on_hand => 0) + create(:product, :supplier => d5).delete + + Enterprise.with_supplied_active_products_on_hand.sort.should == [d1, d2] + Enterprise.with_supplied_active_products_on_hand.distinct_count.should == 2 + end end describe "distributing_product" do