From 94d50f220f698f5bc9da880ceab9b684fb017ac0 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 31 Oct 2014 11:59:46 +1100 Subject: [PATCH] Display an error message to admin when there are hubs in order cycles that are not ready for checkout --- .../spree/admin/base_controller_decorator.rb | 35 +++++++++++++++++++ app/models/enterprise.rb | 10 ++++++ .../spree/admin/base_controller_spec.rb | 23 ++++++++++++ spec/features/admin/order_cycles_spec.rb | 30 ++++++++++++++++ spec/models/enterprise_spec.rb | 26 ++++++++++++++ 5 files changed, 124 insertions(+) diff --git a/app/controllers/spree/admin/base_controller_decorator.rb b/app/controllers/spree/admin/base_controller_decorator.rb index 7199ca4dec..85904590c3 100644 --- a/app/controllers/spree/admin/base_controller_decorator.rb +++ b/app/controllers/spree/admin/base_controller_decorator.rb @@ -1,4 +1,16 @@ Spree::Admin::BaseController.class_eval do + before_filter :warn_invalid_order_cycles + + # Warn the user when they have an active order cycle with hubs that are not ready + # for checkout (ie. does not have valid shipping and payment methods). + def warn_invalid_order_cycles + distributors = active_distributors_not_ready_for_checkout + + if distributors.any? && flash[:notice].nil? + flash[:notice] = active_distributors_not_ready_for_checkout_message(distributors) + end + end + # Override Spree method # It's a shame Spree doesn't just let CanCan handle this in it's own way def authorize_admin @@ -23,4 +35,27 @@ Spree::Admin::BaseController.class_eval do redirect_to root_path(anchor: "login?after_login=#{request.env['PATH_INFO']}") end end + + + private + + def active_distributors_not_ready_for_checkout + ocs = OrderCycle.managed_by(spree_current_user).active + distributors = ocs.map(&:distributors).flatten.uniq + Enterprise.where('id IN (?)', distributors).not_ready_for_checkout + end + + def active_distributors_not_ready_for_checkout_message(distributors) + distributor_names = distributors.map(&:name).join ', ' + + if distributors.count > 1 + "The hubs #{distributor_names} are listed in an active order cycle, " + + "but do not have valid shipping and payment methods. " + + "Until you set these up, customers will not be able to shop at these hubs." + else + "The hub #{distributor_names} is listed in an active order cycle, " + + "but does not have valid shipping and payment methods. " + + "Until you set these up, customers will not be able to shop at this hub." + end + end end diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index fc95d32613..31ae2b3ee2 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -72,6 +72,16 @@ class Enterprise < ActiveRecord::Base merge(Spree::PaymentMethod.available). select('DISTINCT enterprises.*') } + scope :not_ready_for_checkout, lambda { + # When ready_for_checkout is empty, ActiveRecord generates the SQL: + # id NOT IN (NULL) + # I would have expected this to return all rows, but instead it returns none. To + # work around this, we use the "OR ?=0" clause to return all rows when there are + # no enterprises ready for checkout. + where('id NOT IN (?) OR ?=0', + Enterprise.ready_for_checkout, + Enterprise.ready_for_checkout.count) + } scope :is_primary_producer, where(:is_primary_producer => true) scope :is_distributor, where('sells != ?', 'none') scope :supplying_variant_in, lambda { |variants| joins(:supplied_products => :variants_including_master).where('spree_variants.id IN (?)', variants).select('DISTINCT enterprises.*') } diff --git a/spec/controllers/spree/admin/base_controller_spec.rb b/spec/controllers/spree/admin/base_controller_spec.rb index 0985d733a7..c56b3ae0db 100644 --- a/spec/controllers/spree/admin/base_controller_spec.rb +++ b/spec/controllers/spree/admin/base_controller_spec.rb @@ -12,4 +12,27 @@ describe Spree::Admin::BaseController do get :index response.should redirect_to root_path(anchor: "login?after_login=/anonymous") end + + describe "displaying error messages for active distributors not ready for checkout" do + it "generates an error message when there is one distributor" do + distributor = double(:distributor, name: 'My Hub') + controller. + send(:active_distributors_not_ready_for_checkout_message, [distributor]). + should == + "The hub My Hub is listed in an active order cycle, " + + "but does not have valid shipping and payment methods. " + + "Until you set these up, customers will not be able to shop at this hub." + end + + it "generates an error message when there are several distributors" do + d1 = double(:distributor, name: 'Hub One') + d2 = double(:distributor, name: 'Hub Two') + controller. + send(:active_distributors_not_ready_for_checkout_message, [d1, d2]). + should == + "The hubs Hub One, Hub Two are listed in an active order cycle, " + + "but do not have valid shipping and payment methods. " + + "Until you set these up, customers will not be able to shop at these hubs." + end + end end diff --git a/spec/features/admin/order_cycles_spec.rb b/spec/features/admin/order_cycles_spec.rb index 45f2ea115a..e5d8769982 100644 --- a/spec/features/admin/order_cycles_spec.rb +++ b/spec/features/admin/order_cycles_spec.rb @@ -439,6 +439,36 @@ feature %q{ end + describe "ensuring that hubs in order cycles have valid shipping and payment methods" do + context "when they don't" do + let(:hub) { create(:distributor_enterprise) } + let!(:oc) { create(:simple_order_cycle, distributors: [hub]) } + + it "displays a warning on the dashboard" do + login_to_admin_section + page.should have_content "The hub #{hub.name} is listed in an active order cycle, but does not have valid shipping and payment methods. Until you set these up, customers will not be able to shop at this hub." + end + + it "displays a warning on the order cycles screen" do + login_to_admin_section + visit admin_order_cycles_path + page.should have_content "The hub #{hub.name} is listed in an active order cycle, but does not have valid shipping and payment methods. Until you set these up, customers will not be able to shop at this hub." + end + end + + context "when they do" do + let(:hub) { create(:distributor_enterprise) } + let!(:shipping_method) { create(:shipping_method, distributors: [hub]) } + let!(:payment_method) { create(:payment_method, distributors: [hub]) } + let!(:oc) { create(:simple_order_cycle, distributors: [hub]) } + + it "does not display the warning on the dashboard" do + login_to_admin_section + page.should_not have_content "does not have valid shipping and payment methods" + end + end + end + context "as an enterprise user" do let!(:supplier_managed) { create(:supplier_enterprise, name: 'Managed supplier') } diff --git a/spec/models/enterprise_spec.rb b/spec/models/enterprise_spec.rb index f6cc94b8ce..65571e3da7 100644 --- a/spec/models/enterprise_spec.rb +++ b/spec/models/enterprise_spec.rb @@ -193,6 +193,32 @@ describe Enterprise do end end + describe "not_ready_for_checkout" do + let!(:e) { create(:enterprise) } + + it "shows enterprises with no payment methods" do + create(:shipping_method, distributors: [e]) + Enterprise.not_ready_for_checkout.should include e + end + + it "shows enterprises with no shipping methods" do + create(:payment_method, distributors: [e]) + Enterprise.not_ready_for_checkout.should include e + end + + it "shows enterprises with unavailable payment methods" do + create(:shipping_method, distributors: [e]) + create(:payment_method, distributors: [e], active: false) + Enterprise.not_ready_for_checkout.should include e + end + + it "does not show enterprises with available payment and shipping methods" do + create(:shipping_method, distributors: [e]) + create(:payment_method, distributors: [e]) + Enterprise.not_ready_for_checkout.should_not include e + end + end + describe "distributors_with_active_order_cycles" do it "finds active distributors by order cycles" do s = create(:supplier_enterprise)