From 1e817af5aac0fe778e06608bab7bfd99148d2199 Mon Sep 17 00:00:00 2001 From: Cillian O'Ruanaidh Date: Wed, 8 Jun 2022 21:04:37 +0100 Subject: [PATCH] Validate deleting a shipping method or switching it to backoffice only doesn't invalidate any order cycles --- app/models/spree/shipping_method.rb | 31 ++++++++++ config/locales/en.yml | 5 ++ spec/models/spree/shipping_method_spec.rb | 69 +++++++++++++++++++++++ 3 files changed, 105 insertions(+) diff --git a/app/models/spree/shipping_method.rb b/app/models/spree/shipping_method.rb index 4e9fe38825..7a7eb44487 100644 --- a/app/models/spree/shipping_method.rb +++ b/app/models/spree/shipping_method.rb @@ -30,8 +30,11 @@ module Spree validates :name, presence: true validate :distributor_validation validate :at_least_one_shipping_category + validate :switching_to_backoffice_only_wont_leave_order_cycles_without_shipping_methods validates :display_on, inclusion: { in: DISPLAY_ON_OPTIONS.values }, allow_nil: true + before_destroy :check_destroy_wont_leave_order_cycles_without_shipping_methods + after_save :touch_distributors scope :managed_by, lambda { |user| @@ -112,6 +115,20 @@ module Spree private + def no_active_or_upcoming_non_simple_order_cycles_with_only_one_shipping_method? + return true if new_record? + + OrderCycle.active.or(OrderCycle.upcoming).joins(:coordinator, :shipping_methods).where(" + sells != 'own' AND + spree_shipping_methods.id = ? AND + NOT EXISTS( + SELECT 1 + FROM order_cycle_shipping_methods + WHERE order_cycle_id = order_cycles.id AND + shipping_method_id != ? + )", id, id).none? + end + def at_least_one_shipping_category return unless shipping_categories.empty? @@ -127,5 +144,19 @@ module Spree def distributor_validation validates_with DistributorsValidator end + + def check_destroy_wont_leave_order_cycles_without_shipping_methods + return if no_active_or_upcoming_non_simple_order_cycles_with_only_one_shipping_method? + + errors.add(:base, :destroy_leaves_order_cycles_without_shipping_methods) + throw :abort + end + + def switching_to_backoffice_only_wont_leave_order_cycles_without_shipping_methods + return if frontend? || + no_active_or_upcoming_non_simple_order_cycles_with_only_one_shipping_method? + + errors.add(:base, :switching_to_backoffice_only_leaves_order_cycles_without_shipping_methods) + end end end diff --git a/config/locales/en.yml b/config/locales/en.yml index de8bdb19ac..7c25622af2 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -73,6 +73,11 @@ en: attributes: base: card_expired: "has expired" + spree/shipping_method: + attributes: + base: + switching_to_backoffice_only_leaves_order_cycles_without_shipping_methods: Unable to switch to backoffice only, some open or upcoming order cycles would be left without any shipping methods + destroy_leaves_order_cycles_without_shipping_methods: Unable to delete, some open or upcoming order cycles would be left without any shipping methods order_cycle: attributes: base: diff --git a/spec/models/spree/shipping_method_spec.rb b/spec/models/spree/shipping_method_spec.rb index 14d7333120..8c030fa02d 100644 --- a/spec/models/spree/shipping_method_spec.rb +++ b/spec/models/spree/shipping_method_spec.rb @@ -180,6 +180,31 @@ module Spree it { expect(shipping_method).to be_valid } end end + + context "when it is being changed to backoffice only" do + let!(:order_cycle) { create(:distributor_order_cycle, with_distributor_and_shipping_method: true) } + let(:distributor) { order_cycle.distributors.first } + let(:shipping_method) { order_cycle.shipping_methods.first } + + context "when the shipping method is the only shipping method on a distributor order cycle" do + it "should not be valid" do + shipping_method.display_on = "back_end" + + expect(shipping_method).not_to be_valid + expect(shipping_method.errors.to_a).to eq ["Unable to switch to backoffice only, some open or upcoming order cycles would be left without any shipping methods"] + end + end + + context "when the order cycles the shipping method is attached to has other valid shipping methods" do + it "is is valid" do + order_cycle.shipping_methods << create(:shipping_method, distributors: [distributor]) + + shipping_method.display_on = "back_end" + + expect(shipping_method).to be_valid + end + end + end end # Regression test for Spree #4320 @@ -213,5 +238,49 @@ module Spree expect(shipping_method.shipments).to include(shipment) end end + + context "#destroy" do + let(:shipping_method) { create(:shipping_method) } + let(:distributor) { create(:distributor_enterprise, shipping_methods: [shipping_method]) } + let!(:order_cycle) { create(:distributor_order_cycle, distributors: [distributor], shipping_methods: [shipping_method]) } + + context "when the shipping method is the only shipping method on a distributor order cycle" do + it "can be deleted if the order cycle is closed" do + order_cycle.update!(orders_close_at: 1.minute.ago) + + shipping_method.destroy + + expect(shipping_method).to be_deleted + end + + it "cannot be deleted if the order cycle is active" do + order_cycle.update!(orders_open_at: 1.day.ago, orders_close_at: 1.week.from_now) + + shipping_method.destroy + + expect(shipping_method).not_to be_deleted + expect(shipping_method.errors.to_a).to eq ["Unable to delete, some open or upcoming order cycles would be left without any shipping methods"] + end + + it "cannot be deleted if the order cycle is upcoming" do + order_cycle.update!(orders_open_at: 1.day.from_now, orders_close_at: 1.week.from_now) + + shipping_method.destroy + + expect(shipping_method).not_to be_deleted + expect(shipping_method.errors.to_a).to eq ["Unable to delete, some open or upcoming order cycles would be left without any shipping methods"] + end + end + + context "when the order cycles the shipping method is attached to has other valid shipping methods" do + it "can be deleted" do + order_cycle.shipping_methods << create(:shipping_method, distributors: [distributor]) + + shipping_method.destroy + + expect(shipping_method).to be_deleted + end + end + end end end