From f4a4f7c9ff768f4a99b30c8541b430b19a0ac808 Mon Sep 17 00:00:00 2001 From: Cillian O'Ruanaidh Date: Fri, 24 Jun 2022 15:33:15 +0100 Subject: [PATCH] Remove validations checking order cycle has shipping methods Instead we will make sure the order cycle is not available on the shopfront if it is doesn't have valid shipping methods. This will preven the issue where if one distributor deletes their shipping method, we don't want to invalidate the order cycle for all other distributors. Co-authored-by: Maikel --- app/controllers/base_controller.rb | 6 +--- app/models/order_cycle.rb | 26 -------------- spec/models/order_cycle_spec.rb | 58 ------------------------------ 3 files changed, 1 insertion(+), 89 deletions(-) diff --git a/app/controllers/base_controller.rb b/app/controllers/base_controller.rb index 626307db49..a04725416a 100644 --- a/app/controllers/base_controller.rb +++ b/app/controllers/base_controller.rb @@ -15,12 +15,8 @@ class BaseController < ApplicationController private - def all_distributor_order_cycles_invalid? - OrderCycle.with_distributor(@distributor).active.all?(&:invalid?) - end - def set_order_cycles - if !@distributor.ready_for_checkout? || all_distributor_order_cycles_invalid? + if !@distributor.ready_for_checkout? @order_cycles = OrderCycle.where('false') return end diff --git a/app/models/order_cycle.rb b/app/models/order_cycle.rb index 8c354e5e97..ed72e8912f 100644 --- a/app/models/order_cycle.rb +++ b/app/models/order_cycle.rb @@ -36,8 +36,6 @@ class OrderCycle < ApplicationRecord 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_order_cycle_shipping_methods validate :orders_close_at_after_orders_open_at? preference :product_selection_from_coordinator_inventory_only, :boolean, default: false @@ -309,30 +307,6 @@ class OrderCycle < ApplicationRecord private - def all_distributors_have_at_least_one_shipping_method? - distributors.all? do |distributor| - (distributor.shipping_method_ids & selected_shipping_method_ids).any? - end - end - - def at_least_one_shipping_method_selected_for_each_distributor - return if selected_shipping_methods.none? || - coordinator.nil? || - simple? || - all_distributors_have_at_least_one_shipping_method? - - errors.add(:base, :at_least_one_shipping_method_per_distributor) - end - - def no_invalid_order_cycle_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 diff --git a/spec/models/order_cycle_spec.rb b/spec/models/order_cycle_spec.rb index 6d6c392399..5d708d7751 100644 --- a/spec/models/order_cycle_spec.rb +++ b/spec/models/order_cycle_spec.rb @@ -20,64 +20,6 @@ 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.selected_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.selected_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_order_cycle_shipping_methods" do - context "when a order cycle 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 - shipping_method = create(:shipping_method) - distributor = create(:distributor_enterprise, shipping_methods: [shipping_method]) - order_cycle = create(:distributor_order_cycle, distributors: [distributor]) - order_cycle.selected_shipping_methods << shipping_method - - 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 - it "has a coordinator and associated fees" do oc = create(:simple_order_cycle)