Merge branch 'supplied-active-products-on-hand'

Conflicts:
	app/controllers/application_controller.rb
	app/models/enterprise.rb
	app/views/spree/products/_source_sidebar.html.haml
	spec/features/consumer/distributors_spec.rb
	spec/models/enterprises_spec.rb

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;
+  }
 }
 
 

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

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
 

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

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

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

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

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 => "<p>Hello, world!</p>")
@@ -45,5 +98,4 @@ feature %q{
     page.should have_content p1.name
     page.should_not have_content p2.name
   end
-
 end

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

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