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/assets/javascripts/admin/controllers/enterprises_dashboard_controller.js.coffee b/app/assets/javascripts/admin/controllers/enterprises_dashboard_controller.js.coffee deleted file mode 100644 index 60315d30c1..0000000000 --- a/app/assets/javascripts/admin/controllers/enterprises_dashboard_controller.js.coffee +++ /dev/null @@ -1,2 +0,0 @@ -angular.module("ofn.admin").controller "enterprisesDashboardCtrl", ($scope) -> - $scope.activeTab = "hubs" 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/app/models/enterprise.rb b/app/models/enterprise.rb index 82618e4e2e..52ffa73ad1 100644 --- a/app/models/enterprise.rb +++ b/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 diff --git a/app/views/spree/admin/overview/_enterprise_row.html.haml b/app/views/spree/admin/overview/_enterprise_row.html.haml new file mode 100644 index 0000000000..682a50b65c --- /dev/null +++ b/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 + diff --git a/app/views/spree/admin/overview/_enterprises.html.haml b/app/views/spree/admin/overview/_enterprises.html.haml index fbf19fbd43..1ffb68e8cb 100644 --- a/app/views/spree/admin/overview/_enterprises.html.haml +++ b/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 diff --git a/app/views/spree/admin/overview/_enterprises_footer.html.haml b/app/views/spree/admin/overview/_enterprises_footer.html.haml deleted file mode 100644 index c704dfef64..0000000000 --- a/app/views/spree/admin/overview/_enterprises_footer.html.haml +++ /dev/null @@ -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 diff --git a/app/views/spree/admin/overview/_enterprises_hubs_tab.html.haml b/app/views/spree/admin/overview/_enterprises_hubs_tab.html.haml deleted file mode 100644 index dbaff71cfa..0000000000 --- a/app/views/spree/admin/overview/_enterprises_hubs_tab.html.haml +++ /dev/null @@ -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 diff --git a/app/views/spree/admin/overview/_enterprises_none.html.haml b/app/views/spree/admin/overview/_enterprises_none.html.haml deleted file mode 100644 index 72cd90df1a..0000000000 --- a/app/views/spree/admin/overview/_enterprises_none.html.haml +++ /dev/null @@ -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 diff --git a/app/views/spree/admin/overview/_enterprises_producers_tab.html.haml b/app/views/spree/admin/overview/_enterprises_producers_tab.html.haml deleted file mode 100644 index 19cae1e6f8..0000000000 --- a/app/views/spree/admin/overview/_enterprises_producers_tab.html.haml +++ /dev/null @@ -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 diff --git a/app/views/spree/admin/overview/_enterprises_tabs.html.haml b/app/views/spree/admin/overview/_enterprises_tabs.html.haml deleted file mode 100644 index 5f8eb9e3c2..0000000000 --- a/app/views/spree/admin/overview/_enterprises_tabs.html.haml +++ /dev/null @@ -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" diff --git a/config/locales/en.yml b/config/locales/en.yml index 968fd0a06b..285d710f1f 100644 --- a/config/locales/en.yml +++ b/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" 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 diff --git a/spec/features/admin/enterprise_user_spec.rb b/spec/features/admin/enterprise_user_spec.rb index 635b819471..dd24e465d8 100644 --- a/spec/features/admin/enterprise_user_spec.rb +++ b/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' diff --git a/spec/features/admin/overview_spec.rb b/spec/features/admin/overview_spec.rb index c1119a252e..530363929c 100644 --- a/spec/features/admin/overview_spec.rb +++ b/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 diff --git a/spec/models/enterprise_spec.rb b/spec/models/enterprise_spec.rb index 890c1a987d..8fc1b57a0b 100644 --- a/spec/models/enterprise_spec.rb +++ b/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)