diff --git a/app/controllers/spree/admin/base_controller.rb b/app/controllers/spree/admin/base_controller.rb index c1c0179133..73a18752aa 100644 --- a/app/controllers/spree/admin/base_controller.rb +++ b/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}." diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index ff95102781..1deee05a40 100644 --- a/app/models/enterprise.rb +++ b/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 diff --git a/app/models/enterprise_group.rb b/app/models/enterprise_group.rb index 7ffc9d3fb7..4d8ff217f2 100644 --- a/app/models/enterprise_group.rb +++ b/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 diff --git a/app/models/order_cycle.rb b/app/models/order_cycle.rb index b26625fe68..9a30c493e5 100644 --- a/app/models/order_cycle.rb +++ b/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 } diff --git a/app/services/order_cycle_warning.rb b/app/services/order_cycle_warning.rb new file mode 100644 index 0000000000..5f8b532f68 --- /dev/null +++ b/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 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