Merge pull request #2291 from coopdevs/remove-producers-tab-from-dashboard

Remove producers tab from dashboard

.rubocop_todo.yml

@@ -2149,7 +2149,6 @@ Style/IfInsideElse:
   Exclude:
     - 'app/controllers/admin/column_preferences_controller.rb'
     - 'app/controllers/admin/variant_overrides_controller.rb'
-    - 'app/controllers/spree/admin/overview_controller_decorator.rb'
     - 'app/controllers/spree/admin/products_controller_decorator.rb'
 
 # Offense count: 1
@@ -2435,7 +2434,6 @@ Style/RedundantSelf:
 Style/RegexpLiteral:
   Exclude:
     - 'app/controllers/admin/enterprises_controller.rb'
-    - 'app/controllers/spree/admin/overview_controller_decorator.rb'
     - 'app/helpers/groups_helper.rb'
     - 'app/helpers/html_helper.rb'
     - 'app/models/enterprise.rb'

app/assets/javascripts/admin/controllers/enterprises_dashboard_controller.js.coffee

@@ -1,2 +0,0 @@
-angular.module("ofn.admin").controller "enterprisesDashboardCtrl", ($scope) ->
-  $scope.activeTab = "hubs"

app/controllers/spree/admin/overview_controller_decorator.rb

@@ -1,26 +1,74 @@
 Spree::Admin::OverviewController.class_eval do
   def index
-    # TODO was sorted with is_distributor DESC as well, not sure why or how we want ot sort this now
-    @enterprises = Enterprise.managed_by(spree_current_user).order('is_primary_producer ASC, name')
+    @enterprises = Enterprise
+      .managed_by(spree_current_user)
+      .order('is_primary_producer ASC, name')
     @product_count = Spree::Product.active.managed_by(spree_current_user).count
     @order_cycle_count = OrderCycle.active.managed_by(spree_current_user).count
 
-    unspecified = spree_current_user.owned_enterprises.where(sells: 'unspecified')
-    outside_referral = !URI(request.referer.to_s).path.match(/^\/admin/)
-
-    if OpenFoodNetwork::Permissions.new(spree_current_user).manages_one_enterprise? && !spree_current_user.admin?
-      @enterprise = @enterprises.first
-      if outside_referral && unspecified.any?
-        redirect_to main_app.welcome_admin_enterprise_path(@enterprise)
-      else
-        render "single_enterprise_dashboard"
-      end
+    if first_access
+      redirect_to enterprises_path
     else
-      if outside_referral && unspecified.any?
-        redirect_to main_app.admin_enterprises_path
-      else
-        render "multi_enterprise_dashboard"
-      end
+      render dashboard_view
     end
   end
+
+  private
+
+  # Checks whether the user is accessing the admin for the first time
+  #
+  # @return [Boolean]
+  def first_access
+    outside_referral && incomplete_enterprise_registration?
+  end
+
+  # Checks whether the request comes from another admin page or not
+  #
+  # @return [Boolean]
+  def outside_referral
+    !URI(request.referer.to_s).path.match(%r{/admin})
+  end
+
+  # Checks that all of the enterprises owned by the current user have a 'sells'
+  # property specified, which indicates that the registration process has been
+  # completed
+  #
+  # @return [Boolean]
+  def incomplete_enterprise_registration?
+    @incomplete_enterprise_registration ||= spree_current_user
+      .owned_enterprises
+      .where(sells: 'unspecified')
+      .exists?
+  end
+
+  # Returns the appropriate enterprise path for the current user
+  #
+  # @return [String]
+  def enterprises_path
+    if managed_enterprises.size == 1
+      @enterprise = @enterprises.first
+      main_app.welcome_admin_enterprise_path(@enterprise)
+    else
+      main_app.admin_enterprises_path
+    end
+  end
+
+  # Returns the appropriate dashboard view for the current user
+  #
+  # @return [String]
+  def dashboard_view
+    if managed_enterprises.size == 1
+      @enterprise = @enterprises.first
+      :single_enterprise_dashboard
+    else
+      :multi_enterprise_dashboard
+    end
+  end
+
+  # Returns the list of enterprises the current user is manager of
+  #
+  # @return [ActiveRecord::Relation<Enterprise>]
+  def managed_enterprises
+    spree_current_user.enterprises
+  end
 end

app/models/enterprise.rb

@@ -203,14 +203,6 @@ class Enterprise < ActiveRecord::Base
     self.supplied_products.where('count_on_hand > 0').present?
   end
 
-  def supplied_and_active_products_on_hand
-    self.supplied_products.where('spree_products.count_on_hand > 0').active
-  end
-
-  def active_products_in_order_cycles
-    self.supplied_and_active_products_on_hand.in_an_active_order_cycle
-  end
-
   def to_param
     permalink
   end

app/views/spree/admin/overview/_enterprise_row.html.haml

@@ -0,0 +1,33 @@
+%a.sixteen.columns.alpha.list-item{ class: "#{cycle('odd','even')}", href: "#{main_app.edit_admin_enterprise_path(enterprise)}" }
+  %span.five.columns.alpha
+    = enterprise.name
+  %span.symbol.three.columns.centered
+    - if can?(:admin, Spree::PaymentMethod) && enterprise.is_distributor
+      - payment_method_count = enterprise.payment_methods.count
+      - if payment_method_count > 0
+        %span.icon-ok-sign{ 'ofn-with-tip' => "#{pluralize payment_method_count, 'payment method'}" }
+      - else
+        %span.icon-remove-sign{ 'ofn-with-tip' => t('.has_no_payment_methods', enterprise: enterprise.name) }
+    - else
+       
+  %span.symbol.three.columns.centered
+    - if can?(:admin, Spree::ShippingMethod) && enterprise.is_distributor
+      - shipping_method_count = enterprise.shipping_methods.count
+      - if shipping_method_count > 0
+        %span.icon-ok-sign{ 'ofn-with-tip' => "#{pluralize shipping_method_count, 'shipping method'}" }
+      - else
+        %span.icon-remove-sign{ 'ofn-with-tip' => t('.has_no_shipping_methods', enterprise: enterprise.name) }
+    - else
+       
+  %span.symbol.three.columns.centered
+    - if can?(:admin, EnterpriseFee) && enterprise.is_distributor
+      - fee_count = enterprise.enterprise_fees.count
+      - if fee_count > 0
+        %span.icon-ok-sign{ 'ofn-with-tip' => "#{pluralize fee_count, 'fee'}" }
+      - else
+        %span.icon-warning-sign{ 'ofn-with-tip' => t('.has_no_enterprise_fees', enterprise: enterprise.name) }
+    - else
+       
+  %span.two.columns.omega.right
+    %span.icon-arrow-right
+

app/views/spree/admin/overview/_enterprises.html.haml

@@ -1,11 +1,33 @@
-%div.dashboard_item.sixteen.columns.alpha#enterprises{ 'ng-controller' => "enterprisesDashboardCtrl" }
+%div.dashboard_item.sixteen.columns.alpha#enterprises
   = render 'enterprises_header'
 
   - if @enterprises.empty?
-    = render 'enterprises_none'
+    %div.sixteen.columns.alpha.list-item.red
+      %span.text.fifteen.columns.alpha
+        = t "spree_admin_enterprises_none_text"
+      %span.one.columns.omega
+        %span.icon-remove-sign
+    %a.sixteen.columns.alpha.button.bottom.red{ href: "#{main_app.new_admin_enterprise_path}" }
+      = t "spree_admin_enterprises_none_create_a_new_enterprise"
+      %span.icon-arrow-right
 
   - else
-    = render 'enterprises_tabs'
-    = render 'enterprises_hubs_tab'
-    = render 'enterprises_producers_tab'
-    = render 'enterprises_footer'
+    %div.sixteen.columns.alpha.list-title
+      %span.five.columns.alpha
+        = t "spree_admin_enterprises_hubs_name"
+      - if can? :admin, Spree::PaymentMethod
+        %span.centered.three.columns
+          = t(:payment_methods)
+      - if can? :admin, Spree::ShippingMethod
+        %span.centered.three.columns
+          = t "spree_admin_enterprises_shipping_methods"
+      - if can? :admin, EnterpriseFee
+        %span.centered.three.columns
+          = t "spree_admin_enterprises_fees"
+    %div.sixteen.columns.alpha.list
+      - @enterprises.each do |enterprise|
+        = render 'enterprise_row', { enterprise: enterprise }
+
+    %a.sixteen.columns.alpha.button.bottom.blue{ href: "#{main_app.admin_enterprises_path}" }
+      = t "spree_admin_overview_enterprises_footer"
+      %span.icon-arrow-right

app/views/spree/admin/overview/_enterprises_footer.html.haml

@@ -1,3 +0,0 @@
-%a.sixteen.columns.alpha.button.bottom.blue{ href: "#{main_app.admin_enterprises_path}" }
-  = t "spree_admin_overview_enterprises_footer"
-  %span.icon-arrow-right

app/views/spree/admin/overview/_enterprises_hubs_tab.html.haml

@@ -1,47 +0,0 @@
-%div.hubs_tab{ ng: { show: "activeTab == 'hubs'"} }
-  %div.sixteen.columns.alpha.list-title
-    %span.five.columns.alpha
-      = t "spree_admin_enterprises_hubs_name"
-    - if can? :admin, Spree::PaymentMethod
-      %span.centered.three.columns
-        = t(:payment_methods)
-    - if can? :admin, Spree::ShippingMethod
-      %span.centered.three.columns
-        = t "spree_admin_enterprises_shipping_methods"
-    - if can? :admin, EnterpriseFee
-      %span.centered.three.columns
-        = t "spree_admin_enterprises_fees"
-  %div.sixteen.columns.alpha.list
-    - @enterprises.is_distributor.each do |enterprise|
-      %a.sixteen.columns.alpha.list-item{ class: "#{cycle('odd','even')}", href: "#{main_app.edit_admin_enterprise_path(enterprise)}" }
-        %span.five.columns.alpha
-          = enterprise.name
-        %span.symbol.three.columns.centered
-          - if can? :admin, Spree::PaymentMethod
-            - payment_method_count = enterprise.payment_methods.count
-            - if payment_method_count > 0
-              %span.icon-ok-sign{ 'ofn-with-tip' => "#{pluralize payment_method_count, 'payment method'}" }
-            - else
-              %span.icon-remove-sign{ 'ofn-with-tip' => t('.has_no_payment_methods', enterprise: enterprise.name) }
-          - else
-             
-        %span.symbol.three.columns.centered
-          - if can? :admin, Spree::ShippingMethod
-            - shipping_method_count = enterprise.shipping_methods.count
-            - if shipping_method_count > 0
-              %span.icon-ok-sign{ 'ofn-with-tip' => "#{pluralize shipping_method_count, 'shipping method'}" }
-            - else
-              %span.icon-remove-sign{ 'ofn-with-tip' => t('.has_no_shipping_methods', enterprise: enterprise.name) }
-          - else
-             
-        %span.symbol.three.columns.centered
-          - if can? :admin, EnterpriseFee
-            - fee_count = enterprise.enterprise_fees.count
-            - if fee_count > 0
-              %span.icon-ok-sign{ 'ofn-with-tip' => "#{pluralize fee_count, 'fee'}" }
-            - else
-              %span.icon-warning-sign{ 'ofn-with-tip' => t('.has_no_enterprise_fees', enterprise: enterprise.name) }
-          - else
-             
-        %span.two.columns.omega.right
-          %span.icon-arrow-right

app/views/spree/admin/overview/_enterprises_none.html.haml

@@ -1,8 +0,0 @@
-%div.sixteen.columns.alpha.list-item.red
-  %span.text.fifteen.columns.alpha
-    = t "spree_admin_enterprises_none_text"
-  %span.one.columns.omega
-    %span.icon-remove-sign
-%a.sixteen.columns.alpha.button.bottom.red{ href: "#{main_app.new_admin_enterprise_path}" }
-  = t "spree_admin_enterprises_none_create_a_new_enterprise"
-  %span.icon-arrow-right

app/views/spree/admin/overview/_enterprises_producers_tab.html.haml

@@ -1,47 +0,0 @@
-%div.producers_tab{ ng: { show: "activeTab == 'producers'"} }
-  %div.list-title.sixteen.columns.alpha
-    %span.five.columns.alpha
-      = t "spree_admin_enterprises_producers_name"
-    - if can? :admin, Spree::Product
-      %span.centered.three.columns
-        = t "spree_admin_enterprises_producers_total_products"
-      %span.centered.three.columns
-        = t "spree_admin_enterprises_producers_active_products"
-    - if can? :admin, OrderCycle
-      %span.centered.three.columns
-        = t "spree_admin_enterprises_producers_order_cycles"
-  %div.sixteen.columns.alpha.list
-    - @enterprises.is_primary_producer.each do |enterprise|
-      %a.sixteen.columns.alpha.list-item{ class: "#{cycle('odd','even')}", href: "#{main_app.edit_admin_enterprise_path(enterprise)}" }
-
-        %span.five.columns.alpha
-          = enterprise.name
-
-        %span.symbol.three.columns.centered
-          - if can? :admin, Spree::Product
-            %span.one.column.alpha  
-            %span.text-icon.one.column.centered{ class: "#{enterprise.supplied_products.not_deleted.any? ? "green" : "red" }" }
-              = enterprise.supplied_products.not_deleted.count
-            %span.one.column.omega  
-          - else
-             
-        %span.symbol.three.columns.centered
-          - if can? :admin, Spree::Product
-            %span.one.column.alpha  
-            %span.text-icon.one.column.centered{ class: "#{enterprise.supplied_and_active_products_on_hand.any? ? "green" : "red" }" }
-              = enterprise.supplied_and_active_products_on_hand.count
-            %span.one.column.omega  
-          - else
-             
-
-        %span.symbol.three.columns.centered
-          - if can? :admin, OrderCycle
-            %span.one.column.alpha  
-            %span.text-icon.one.column.centered{ class: "#{enterprise.active_products_in_order_cycles.any? ? "green" : "orange" }" }
-              = enterprise.active_products_in_order_cycles.count
-            %span.one.column.omega  
-          - else
-             
-
-        %span.two.columns.omega.right
-          %span.icon-arrow-right

app/views/spree/admin/overview/_enterprises_tabs.html.haml

@@ -1,5 +0,0 @@
-%div.sixteen.columns.alpha.tabs
-  %div.dashboard_tab.eight.columns.alpha.blue{ ng: { class: "{selected: activeTab == 'hubs'}", click: "activeTab = 'hubs'" } }
-    = t "spree_admin_enterprises_tabs_hubs"
-  %div.dashboard_tab.eight.columns.omega.blue{ ng: { class: "{selected: activeTab == 'producers'}", click: "activeTab = 'producers'" } }
-    = t "spree_admin_enterprises_tabs_producers"

config/locales/en.yml

@@ -1935,13 +1935,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using
   spree_admin_enterprises_fees: "Enterprise Fees"
   spree_admin_enterprises_none_create_a_new_enterprise: "CREATE A NEW ENTERPRISE"
   spree_admin_enterprises_none_text: "You don't have any enterprises yet"
-  spree_admin_enterprises_producers_name: "Name"
-  spree_admin_enterprises_producers_total_products: "Total Products"
-  spree_admin_enterprises_producers_active_products: "Active Products"
-  spree_admin_enterprises_producers_order_cycles: "Products in OCs"
-  spree_admin_enterprises_producers_order_cycles_title: ""
   spree_admin_enterprises_tabs_hubs: "HUBS"
-  spree_admin_enterprises_tabs_producers: "PRODUCERS"
   spree_admin_enterprises_producers_manage_products: "MANAGE PRODUCTS"
   spree_admin_enterprises_any_active_products_text: "You don't have any active products."
   spree_admin_enterprises_create_new_product: "CREATE A NEW PRODUCT"

spec/controllers/spree/admin/overview_controller_spec.rb

@@ -2,18 +2,20 @@ require 'spec_helper'
 
 describe Spree::Admin::OverviewController, type: :controller do
   include AuthenticationWorkflow
-  context "loading overview" do
-    let(:user) { create_enterprise_user(enterprise_limit: 2) }
 
+  describe "#index" do
     before do
-      controller.stub spree_current_user: user
+      allow(controller).to receive(:spree_current_user).and_return(user)
     end
 
     context "when user owns only one enterprise" do
+      let(:user) { create_enterprise_user }
       let!(:enterprise) { create(:distributor_enterprise, owner: user) }
 
       context "when the referer is not an admin page" do
-        before { @request.env['HTTP_REFERER'] = 'http://test.com/some_other_path' }
+        before do
+          @request.env['HTTP_REFERER'] = 'http://test.com/not_admin_path'
+        end
 
         context "and the enterprise has sells='unspecified'" do
           before do
@@ -22,14 +24,15 @@ describe Spree::Admin::OverviewController, type: :controller do
 
           it "redirects to the welcome page for the enterprise" do
             spree_get :index
-            response.should redirect_to welcome_admin_enterprise_path(enterprise)
+            expect(response)
+              .to redirect_to welcome_admin_enterprise_path(enterprise)
           end
         end
 
         context "and the enterprise does not have sells='unspecified'" do
           it "renders the single enterprise dashboard" do
             spree_get :index
-            response.should render_template "single_enterprise_dashboard"
+            expect(response).to render_template :single_enterprise_dashboard
           end
         end
       end
@@ -39,17 +42,21 @@ describe Spree::Admin::OverviewController, type: :controller do
 
         it "renders the single enterprise dashboard" do
           spree_get :index
-          response.should render_template "single_enterprise_dashboard"
+          expect(response).to render_template :single_enterprise_dashboard
         end
       end
     end
 
     context "when user owns multiple enterprises" do
+      let(:user) { create_enterprise_user(enterprise_limit: 2) }
+
       let!(:enterprise1) { create(:distributor_enterprise, owner: user) }
-      let!(:enterprise2) { create(:distributor_enterprise, owner: user) }
+      before { create(:distributor_enterprise, owner: user) }
 
       context "when the referer is not an admin page" do
-        before { @request.env['HTTP_REFERER'] = 'http://test.com/some_other_path' }
+        before do
+          @request.env['HTTP_REFERER'] = 'http://test.com/not_admin_path'
+        end
 
         context "and at least one owned enterprise has sells='unspecified'" do
           before do
@@ -58,14 +65,14 @@ describe Spree::Admin::OverviewController, type: :controller do
 
           it "redirects to the enterprises index" do
             spree_get :index
-            response.should redirect_to admin_enterprises_path
+            expect(response).to redirect_to admin_enterprises_path
           end
         end
 
         context "and no owned enterprises have sells='unspecified'" do
           it "renders the multiple enterprise dashboard" do
             spree_get :index
-            response.should render_template "multi_enterprise_dashboard"
+            expect(response).to render_template :multi_enterprise_dashboard
           end
         end
       end
@@ -75,7 +82,7 @@ describe Spree::Admin::OverviewController, type: :controller do
 
         it "renders the multiple enterprise dashboard" do
           spree_get :index
-          response.should render_template "multi_enterprise_dashboard"
+          expect(response).to render_template :multi_enterprise_dashboard
         end
       end
     end

spec/features/admin/enterprise_user_spec.rb

@@ -88,11 +88,6 @@ feature %q{
       end
 
       it "shows me enterprise product info but not payment methods, shipping methods or enterprise fees" do
-        # Producer product info
-        page.should have_selector '.producers_tab span', text: 'Total Products'
-        page.should have_selector '.producers_tab span', text: 'Active Products'
-        page.should_not have_selector '.producers_tab span', text: 'Products in OCs'
-
         # Payment methods, shipping methods, enterprise fees
         page.should_not have_selector '.hubs_tab span', text: 'Payment Methods'
         page.should_not have_selector '.hubs_tab span', text: 'Shipping Methods'

spec/features/admin/overview_spec.rb

@@ -56,19 +56,24 @@ feature %q{
     context "with multiple enterprises" do
       let(:d1) { create(:distributor_enterprise) }
       let(:d2) { create(:distributor_enterprise) }
+      let(:non_distributor_enterprise) { create(:enterprise, sells: 'none') }
 
-      before :each do
+      before do
         @enterprise_user.enterprise_roles.build(enterprise: d1).save
         @enterprise_user.enterprise_roles.build(enterprise: d2).save
+        @enterprise_user
+          .enterprise_roles.build(enterprise: non_distributor_enterprise).save
       end
 
       it "displays information about the enterprise" do
         visit '/admin'
-        page.should have_selector ".dashboard_item#enterprises h3", text: "My Enterprises"
-        page.should have_selector ".dashboard_item#products"
-        page.should have_selector ".dashboard_item#order_cycles"
-        page.should have_selector ".dashboard_item#enterprises .list-item", text: d1.name
-        page.should have_selector ".dashboard_item#enterprises .button.bottom", text: "MANAGE MY ENTERPRISES"
+
+        expect(page).to have_selector ".dashboard_item#enterprises h3", text: "My Enterprises"
+        expect(page).to have_selector ".dashboard_item#products"
+        expect(page).to have_selector ".dashboard_item#order_cycles"
+        expect(page).to have_selector ".dashboard_item#enterprises .list-item", text: d1.name
+        expect(page).to have_selector ".dashboard_item#enterprises .list-item", text: non_distributor_enterprise.name
+        expect(page).to have_selector ".dashboard_item#enterprises .button.bottom", text: "MANAGE MY ENTERPRISES"
       end
 
       context "but no products or order cycles" do

spec/models/enterprise_spec.rb

@@ -551,16 +551,6 @@ describe Enterprise do
     end
   end
 
-  describe "supplied_and_active_products_on_hand" do
-    it "find only active products which are in stock" do
-      supplier = create(:supplier_enterprise)
-      inactive_product = create(:product, supplier:  supplier, on_hand: 1, available_on: Date.tomorrow)
-      out_of_stock_product = create(:product, supplier:  supplier, on_hand: 0, available_on: Date.yesterday)
-      p1 = create(:product, supplier: supplier, on_hand: 1, available_on: Date.yesterday)
-      supplier.supplied_and_active_products_on_hand.should == [p1]
-    end
-  end
-
   describe "finding variants distributed by the enterprise" do
     it "finds master and other variants" do
       d = create(:distributor_enterprise)