Merge pull request #4667 from Matt-Yorkley/rails-4-prep

Rails 4 prep

app/controllers/spree/admin/base_controller.rb

@@ -15,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
@@ -93,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('enterprises.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
-          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
@@ -121,13 +102,17 @@ module Spree
 
       def render_as_json(data, options = {})
         ams_prefix = options.delete :ams_prefix
-        if [Array, ActiveRecord::Relation].include? data.class
+        if each_serializer_required?(data)
           render options.merge(json: data, each_serializer: serializer(ams_prefix))
         else
           render options.merge(json: data, serializer: serializer(ams_prefix))
         end
       end
 
+      def each_serializer_required?(data)
+        ['Array', 'ActiveRecord::Relation'].include?(data.class.name)
+      end
+
       def serializer(ams_prefix)
         unless ams_prefix.nil? || ams_prefix_whitelist.include?(ams_prefix.to_sym)
           raise "Suffix '#{ams_prefix}' not found in ams_prefix_whitelist for #{self.class.name}."

app/models/enterprise.rb

@@ -25,7 +25,8 @@ class Enterprise < ActiveRecord::Base
   has_many :relationships_as_child, class_name: 'EnterpriseRelationship',
                                     foreign_key: 'child_id',
                                     dependent: :destroy
-  has_and_belongs_to_many :groups, class_name: 'EnterpriseGroup'
+  has_and_belongs_to_many :groups, join_table: 'enterprise_groups_enterprises',
+                                   class_name: 'EnterpriseGroup'
   has_many :producer_properties, foreign_key: 'producer_id'
   has_many :properties, through: :producer_properties
   has_many :supplied_products, class_name: 'Spree::Product',
@@ -115,7 +116,10 @@ class Enterprise < ActiveRecord::Base
   scope :not_ready_for_checkout, lambda {
     # When ready_for_checkout is empty, return all rows when there are no enterprises ready for
     # checkout.
-    ready_enterprises = Enterprise.ready_for_checkout.select('enterprises.id')
+    ready_enterprises = Enterprise.ready_for_checkout.
+      except(:select).
+      select('DISTINCT enterprises.id')
+
     if ready_enterprises.present?
       where("enterprises.id NOT IN (?)", ready_enterprises)
     else

app/models/enterprise_group.rb

@@ -5,7 +5,7 @@ class EnterpriseGroup < ActiveRecord::Base
   include PermalinkGenerator
   acts_as_list
 
-  has_and_belongs_to_many :enterprises
+  has_and_belongs_to_many :enterprises, join_table: 'enterprise_groups_enterprises'
   belongs_to :owner, class_name: 'Spree::User', foreign_key: :owner_id, inverse_of: :owned_groups
   belongs_to :address, class_name: 'Spree::Address'
   accepts_nested_attributes_for :address

app/models/order_cycle.rb

@@ -63,7 +63,7 @@ class OrderCycle < ActiveRecord::Base
     if user.has_spree_role?('admin')
       scoped
     else
-      where('coordinator_id IN (?)', user.enterprises.map(&:id))
+      where(coordinator_id: user.enterprises)
     end
   }
 

app/services/order_cycle_warning.rb

@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+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