Add :shipping_methods association to OrderCyle and validations

app/models/order_cycle.rb

@@ -22,17 +22,23 @@ class OrderCycle < ApplicationRecord
 
   has_many :suppliers, -> { distinct }, source: :sender, through: :cached_incoming_exchanges
   has_many :distributors, -> { distinct }, source: :receiver, through: :cached_outgoing_exchanges
-
   has_many :order_cycle_schedules
   has_many :schedules, through: :order_cycle_schedules
+  has_many :order_cycle_shipping_methods
+  has_many :shipping_methods, class_name: "Spree::ShippingMethod",
+                              through: :order_cycle_shipping_methods
   has_paper_trail meta: { custom_data: proc { |order_cycle| order_cycle.schedule_ids.to_s } }
 
   attr_accessor :incoming_exchanges, :outgoing_exchanges
 
+  attribute :validate_shipping_methods, :boolean, default: true
+
   before_update :reset_processed_at, if: :will_save_change_to_orders_close_at?
   after_save :sync_subscriptions, if: :opening?
 
   validates :name, :coordinator_id, presence: true
+  validate :at_least_one_shipping_method_selected_for_each_distributor
+  validate :no_invalid_shipping_methods
   validate :orders_close_at_after_orders_open_at?
 
   preference :product_selection_from_coordinator_inventory_only, :boolean, default: false
@@ -159,8 +165,12 @@ class OrderCycle < ApplicationRecord
     oc.preferred_product_selection_from_coordinator_inventory_only = preferred_product_selection_from_coordinator_inventory_only
     # rubocop:enable Layout/LineLength
     oc.schedule_ids = schedule_ids
+    oc.shipping_methods_customisable = true
     oc.save!
+    oc.validate_shipping_methods = false
     exchanges.each { |e| e.clone!(oc) }
+    oc.validate_shipping_methods = true
+    oc.shipping_method_ids = shipping_method_ids
     sync_subscriptions
     oc.reload
   end
@@ -280,6 +290,29 @@ class OrderCycle < ApplicationRecord
 
   private
 
+  def all_distributors_have_at_least_one_shipping_method?
+    distributors.all? { |distributor| (distributor.shipping_method_ids & shipping_method_ids).any? }
+  end
+
+  def at_least_one_shipping_method_selected_for_each_distributor
+    return if !validate_shipping_methods? ||
+              coordinator.nil? ||
+              simple? ||
+              !shipping_methods_customisable? ||
+              all_distributors_have_at_least_one_shipping_method?
+
+    errors.add(:base, :at_least_one_shipping_method_per_distributor)
+  end
+
+  def no_invalid_shipping_methods
+    return if order_cycle_shipping_methods.all?(&:valid?)
+
+    errors.add(
+      :base,
+      order_cycle_shipping_methods.map(&:errors).map(&:to_a).flatten.uniq.to_sentence
+    )
+  end
+
   def opening?
     (open? || upcoming?) && saved_change_to_orders_close_at? && was_closed?
   end

config/locales/en.yml

@@ -75,6 +75,8 @@ en:
               card_expired: "has expired"
         order_cycle:
           attributes:
+            base:
+              at_least_one_shipping_method_per_distributor: "You need to select at least one shipping method for each distributor"
             orders_close_at:
               after_orders_open_at: must be after open date
         order_cycle_shipping_method:

spec/factories/order_cycle_factory.rb

@@ -72,12 +72,15 @@ FactoryBot.define do
     coordinator { Enterprise.is_distributor.first || FactoryBot.create(:distributor_enterprise) }
 
     transient do
+      shipping_methods { [] }
       suppliers { [] }
       distributors { [] }
       variants { [] }
+      with_distributor_and_shipping_method { false }
     end
 
     after(:create) do |oc, proxy|
+
       # Incoming Exchanges
       proxy.suppliers.each.with_index do |supplier, i|
         ex = create(:exchange, order_cycle: oc,
@@ -88,6 +91,10 @@ FactoryBot.define do
         proxy.variants.each { |v| ex.variants << v }
       end
 
+      if proxy.with_distributor_and_shipping_method
+        proxy.distributors << oc.coordinator if proxy.distributors.empty?
+      end
+
       # Outgoing Exchanges
       proxy.distributors.each.with_index do |distributor, i|
         ex = create(:exchange, order_cycle: oc,
@@ -98,6 +105,26 @@ FactoryBot.define do
                                pickup_instructions: "instructions #{i}")
         proxy.variants.each { |v| ex.variants << v }
       end
+
+      if proxy.with_distributor_and_shipping_method || proxy.shipping_methods.any?
+        oc.reload # so outgoing exchanges/distributors attached above are present
+        distributor = oc.distributors.first
+        if proxy.shipping_methods.empty?
+          proxy.shipping_methods << create(:shipping_method, distributors: [distributor])
+        else
+          proxy.shipping_methods.each do |shipping_method|
+            # ensure shipping methods belong to a distributor on the order cycle
+            if !shipping_method.distributors.include?(distributor)
+              shipping_method.distributors << distributor
+            end
+          end
+        end
+        oc.shipping_methods = proxy.shipping_methods
+      end
+
+      if proxy.distributors.any? || proxy.shipping_methods.any?
+        oc.reload # so shipping methods attached above are present
+      end
     end
   end
 

spec/models/order_cycle_spec.rb

@@ -20,6 +20,72 @@ describe OrderCycle do
     expect(oc).to_not be_valid
   end
 
+  describe "#at_least_one_shipping_method_selected_for_each_distributor" do
+    context "distributor order cycle i.e. :sells is 'any'" do
+      context "when multiple distributors have been added to the order cycle already" do
+        it "is valid when adding a shipping method for *all* distributors" do
+          distributor_i = create(:distributor_enterprise)
+          distributor_ii = create(:distributor_enterprise)
+          distributor_i_shipping_method = create(:shipping_method, distributors: [distributor_i])
+          distributor_ii_shipping_method = create(:shipping_method, distributors: [distributor_ii])
+          order_cycle = create(:distributor_order_cycle, distributors: [distributor_i, distributor_ii])
+
+          order_cycle.shipping_method_ids = [distributor_i_shipping_method.id, distributor_ii_shipping_method.id]
+
+          expect(order_cycle).to be_valid
+        end
+      end
+
+      it "is not valid when adding a shipping method for *some but not all* distributors" do
+        distributor_i = create(:distributor_enterprise)
+        distributor_ii = create(:distributor_enterprise)
+        distributor_i_shipping_method = create(:shipping_method, distributors: [distributor_i])
+        distributor_ii_shipping_method = create(:shipping_method, distributors: [distributor_i])
+        order_cycle = create(:distributor_order_cycle, distributors: [distributor_i, distributor_ii])
+
+        order_cycle.shipping_method_ids = [distributor_i_shipping_method.id]
+
+        expect(order_cycle).to be_invalid
+        expect(order_cycle.errors.to_a).to eq ["You need to select at least one shipping method for each distributor"]
+      end
+    end
+  end
+
+  describe "#no_invalid_shipping_methods" do
+    context "when a shipping method is not valid" do
+      it "adds a validation error, and it is more meaningful than the default 'Order cycle shipping methods is invalid'" do
+        order_cycle = create(:distributor_order_cycle, with_distributor_and_shipping_method: true)
+        shipping_method = order_cycle.shipping_methods.first
+
+        shipping_method.update_column(:display_on, "back_end")
+
+        expect(order_cycle).to be_invalid
+        expect(order_cycle.errors.to_a).to eq ["Shipping method must be available at checkout"]
+
+        shipping_method.update_column(:display_on, "")
+
+        expect(order_cycle.reload).to be_valid
+      end
+    end
+  end
+
+  describe "#validate_shipping_methods" do
+    it "doesn't skip shipping method validations by default" do
+      order_cycle = create(:distributor_order_cycle, distributors: [create(:distributor_enterprise)])
+
+      expect(order_cycle).to be_invalid
+      expect(order_cycle.errors.to_a).to eq ["You need to select at least one shipping method for each distributor"]
+    end
+
+    it "allows shipping method validations to be skipped because distributors need to be saved before shipping methods" do
+      order_cycle = create(:distributor_order_cycle, distributors: [create(:distributor_enterprise)])
+
+      order_cycle.validate_shipping_methods = false
+
+      expect(order_cycle).to be_valid
+    end
+  end
+
   it "has a coordinator and associated fees" do
     oc = create(:simple_order_cycle)
 
@@ -132,7 +198,7 @@ describe OrderCycle do
   end
 
   it "reports its distributors" do
-    oc = create(:simple_order_cycle)
+    oc = create(:simple_order_cycle, validate_shipping_methods: false)
 
     e1 = create(:exchange, incoming: false,
                            order_cycle: oc, sender: oc.coordinator, receiver: create(:enterprise))
@@ -143,7 +209,7 @@ describe OrderCycle do
   end
 
   it "checks for existance of distributors" do
-    oc = create(:simple_order_cycle)
+    oc = create(:simple_order_cycle, validate_shipping_methods: false)
     d1 = create(:distributor_enterprise)
     d2 = create(:distributor_enterprise)
     create(:exchange, order_cycle: oc, sender: oc.coordinator, receiver: d1, incoming: false)
@@ -506,7 +572,7 @@ describe OrderCycle do
   end
 
   describe "version tracking", versioning: true do
-    let!(:oc) { create(:order_cycle, name: "Original") }
+    let!(:oc) { create(:sells_own_order_cycle, name: "Original") }
 
     it "remembers old versions" do
       expect {
@@ -551,11 +617,12 @@ describe OrderCycle do
   end
 
   describe "syncing subscriptions" do
-    let!(:oc) {
-      create(:simple_order_cycle, orders_open_at: 1.week.ago, orders_close_at: 1.day.ago)
-    }
-    let(:schedule) { create(:schedule, order_cycles: [oc]) }
-    let!(:subscription) { create(:subscription, schedule: schedule, with_items: true) }
+    let!(:subscription) { create(:subscription, with_items: true) }
+    let!(:oc) { subscription.order_cycles.first }
+
+    before do
+      oc.update_columns(orders_open_at: 1.week.ago, orders_close_at: 1.day.ago)
+    end
 
     it "syncs subscriptions when transitioning from closed to open" do
       expect(OrderManagement::Subscriptions::ProxyOrderSyncer).to receive(:new).and_call_original