Refactor BaseController

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

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

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' } }

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