From 5f3abbf00ec10bc1d38bdbbe2e7ad6283a53b1e2 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 13 Jan 2020 11:27:26 +0100 Subject: [PATCH] Refactor BaseController --- .../spree/admin/base_controller.rb | 27 ++------------ app/services/order_cycle_warning.rb | 35 +++++++++++++++++++ .../spree/admin/base_controller_spec.rb | 25 ------------- spec/services/order_cycle_warning_spec.rb | 31 ++++++++++++++++ 4 files changed, 69 insertions(+), 49 deletions(-) create mode 100644 app/services/order_cycle_warning.rb create mode 100644 spec/services/order_cycle_warning_spec.rb diff --git a/app/controllers/spree/admin/base_controller.rb b/app/controllers/spree/admin/base_controller.rb index 7591e1b3ce..73a18752aa 100644 --- a/app/controllers/spree/admin/base_controller.rb +++ b/app/controllers/spree/admin/base_controller.rb @@ -1,4 +1,3 @@ -# rubocop:disable Metrics/ClassLength module Spree module Admin class BaseController < Spree::BaseController @@ -16,11 +15,10 @@ module Spree # 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 + return if flash[:notice].present? - return if distributors.empty? || flash[:notice].present? - - flash[:notice] = active_distributors_not_ready_for_checkout_message(distributors) + warning = OrderCycleWarning.new(spree_current_user).call + flash[:notice] = warning if warning.present? end # This is in Spree::Core::ControllerHelpers::Auth @@ -94,24 +92,6 @@ module Spree private - def active_distributors_not_ready_for_checkout - ocs = OrderCycle.managed_by(spree_current_user).active - distributors = ocs.includes(:distributors).map(&:distributors).flatten.uniq - Enterprise.where(id: distributors.map(&:id)).not_ready_for_checkout - end - - def active_distributors_not_ready_for_checkout_message(distributors) - distributor_names = distributors.map(&:name).join ', ' - - if distributors.count > 1 - I18n.t(:active_distributors_not_ready_for_checkout_message_plural, - distributor_names: distributor_names) - else - I18n.t(:active_distributors_not_ready_for_checkout_message_singular, - distributor_names: distributor_names) - end - end - def html_request? request.format.html? end @@ -145,4 +125,3 @@ module Spree end end end -# rubocop:enable Metrics/ClassLength diff --git a/app/services/order_cycle_warning.rb b/app/services/order_cycle_warning.rb new file mode 100644 index 0000000000..ffe3f9da21 --- /dev/null +++ b/app/services/order_cycle_warning.rb @@ -0,0 +1,35 @@ +class OrderCycleWarning + def initialize(current_user) + @current_user = current_user + end + + def call + distributors = active_distributors_not_ready_for_checkout + + return if distributors.empty? + + active_distributors_not_ready_for_checkout_message(distributors) + end + + private + + attr_reader :current_user + + def active_distributors_not_ready_for_checkout + ocs = OrderCycle.managed_by(current_user).active + distributors = ocs.includes(:distributors).map(&:distributors).flatten.uniq + Enterprise.where(id: distributors.map(&:id)).not_ready_for_checkout + end + + def active_distributors_not_ready_for_checkout_message(distributors) + distributor_names = distributors.map(&:name).join ', ' + + if distributors.count > 1 + I18n.t(:active_distributors_not_ready_for_checkout_message_plural, + distributor_names: distributor_names) + else + I18n.t(:active_distributors_not_ready_for_checkout_message_singular, + distributor_names: distributor_names) + end + end +end diff --git a/spec/controllers/spree/admin/base_controller_spec.rb b/spec/controllers/spree/admin/base_controller_spec.rb index fa6899781d..d5b1015635 100644 --- a/spec/controllers/spree/admin/base_controller_spec.rb +++ b/spec/controllers/spree/admin/base_controller_spec.rb @@ -13,31 +13,6 @@ describe Spree::Admin::BaseController, type: :controller do expect(response).to redirect_to root_path(anchor: "login?after_login=/spree/admin/base") 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') - expect(controller. - send(:active_distributors_not_ready_for_checkout_message, [distributor])). - to eq( - "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') - expect(controller. - send(:active_distributors_not_ready_for_checkout_message, [d1, d2])). - to eq( - "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 - describe "rendering as json ActiveModelSerializer" do context "when data is an object" do let(:data) { { attr: 'value' } } diff --git a/spec/services/order_cycle_warning_spec.rb b/spec/services/order_cycle_warning_spec.rb new file mode 100644 index 0000000000..e2c8457e4b --- /dev/null +++ b/spec/services/order_cycle_warning_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +describe OrderCycleWarning do + let(:user) { create(:user) } + let(:subject) { OrderCycleWarning } + let!(:distributor) { create(:enterprise, owner: user) } + let!(:order_cycle) { create(:simple_order_cycle, distributors: [distributor]) } + + describe "checking if user's managed order cycles have distributors not ready for checkout" do + context "with an invalid distributor" do + it "returns a warning message" do + expect(subject.new(user).call).to eq( + I18n.t(:active_distributors_not_ready_for_checkout_message_singular, + distributor_names: distributor.name) + ) + end + end + + context "with a valid distributor" do + let!(:distributor) { + create(:distributor_enterprise, + shipping_methods: [create(:shipping_method)], + payment_methods: [create(:payment_method)]) + } + + it "returns nil" do + expect(subject.new(user).call).to eq nil + end + end + end +end