diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 4154cf8ac2..3e49ab57d3 100644 --- a/.rubocop_todo.yml +++ b/.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' diff --git a/app/controllers/spree/admin/overview_controller_decorator.rb b/app/controllers/spree/admin/overview_controller_decorator.rb index ffd2569c1f..548631aaf4 100644 --- a/app/controllers/spree/admin/overview_controller_decorator.rb +++ b/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] + def managed_enterprises + spree_current_user.enterprises + end end diff --git a/spec/controllers/spree/admin/overview_controller_spec.rb b/spec/controllers/spree/admin/overview_controller_spec.rb index 7b024f10bf..e445932555 100644 --- a/spec/controllers/spree/admin/overview_controller_spec.rb +++ b/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