Merge pull request #3645 from nebulab/kennyadsl/restore-2208

Add ability to run validations in order updater

core/app/models/spree/order_merger.rb

@@ -56,11 +56,13 @@ def merge!(other_order, user = nil)
       end
 
       set_user(user)
-      persist_merge
+      if order.valid?
+        persist_merge
 
-      # So that the destroy doesn't take out line items which may have been re-assigned
-      other_order.line_items.reload
-      other_order.destroy
+        # So that the destroy doesn't take out line items which may have been re-assigned
+        other_order.line_items.reload
+        other_order.destroy
+      end
     end
 
     private

core/app/models/spree/order_updater.rb

@@ -186,7 +186,7 @@ def update_item_total
     end
 
     def persist_totals
-      order.save!(validate: false)
+      order.save!(validate: Spree::Config.run_order_validations_on_order_updater)
     end
 
     def log_state_change(name)

core/lib/generators/solidus/install/templates/config/initializers/spree.rb.tt

@@ -34,6 +34,12 @@ Spree.config do |config|
   # See https://github.com/solidusio/solidus/pull/3456 for more info.
   config.raise_with_invalid_currency = false
 
+  # Set this configuration to `false` to avoid running validations when
+  # updating an order. Be careful since you can end up having inconsistent
+  # data in your database turning it on.
+  # See https://github.com/solidusio/solidus/pull/3645 for more info.
+  config.run_order_validations_on_order_updater = true
+
   # Permission Sets:
 
   # Uncomment and customize the following line to add custom permission sets

core/lib/spree/app_configuration.rb

@@ -231,6 +231,10 @@ class AppConfiguration < Preferences::Configuration
     #   (default: +['admin']+)
     preference :roles_for_auto_api_key, :array, default: ['admin']
 
+    # @!attribute [rw] run_order_validations_on_order_updater
+    #   @return [Boolean] Whether to run validation when updating an order with the OrderUpdater
+    preference :run_order_validations_on_order_updater, :boolean, default: false
+
     # @!attribute [rw] send_core_emails
     #   @return [Boolean] Whether to send transactional emails (default: true)
     preference :send_core_emails, :boolean, default: true

core/lib/spree/core/engine.rb

@@ -64,6 +64,15 @@ class Engine < ::Rails::Engine
             caller
           )
         end
+
+        if Spree::Config.run_order_validations_on_order_updater != true
+          Spree::Deprecation.warn(
+            'Spree::Config.run_order_validations_on_order_updater set to false is ' \
+            'deprecated and will not be possibile in Solidus 3.0. Please switch this ' \
+            'value to true and check that everything works as expected.',
+            caller
+          )
+        end
       end
 
       # Load in mailer previews for apps to use in development.

core/lib/spree/testing_support/dummy_app.rb

@@ -117,6 +117,7 @@ class Application < ::Rails::Application
 Spree.config do |config|
   config.mails_from = "store@example.com"
   config.raise_with_invalid_currency = false
+  config.run_order_validations_on_order_updater = true
   config.use_combined_first_and_last_name_in_address = true
 
   if ENV['ENABLE_ACTIVE_STORAGE']

core/spec/models/spree/order/checkout_spec.rb

@@ -520,7 +520,8 @@ def assert_state_changed(order, from, to)
       context 'when the line items are not available' do
         before do
           order.line_items << FactoryBot.create(:line_item)
-          order.store = FactoryBot.build(:store)
+          order.store = FactoryBot.create(:store)
+
           Spree::OrderUpdater.new(order).update
 
           order.save!

core/spec/models/spree/order_contents_spec.rb

@@ -301,7 +301,13 @@
   end
 
   context "completed order" do
-    let(:order) { Spree::Order.create! state: 'complete', completed_at: Time.current }
+    let(:order) do
+      Spree::Order.create!(
+        state: 'complete',
+        completed_at: Time.current,
+        email: "test@example.com"
+      )
+    end
 
     before { order.shipments.create! stock_location_id: variant.stock_location_ids.first }
 

core/spec/models/spree/order_updater_spec.rb

@@ -372,7 +372,7 @@ def create_adjustment(label, amount)
     end
 
     context "updating payment state" do
-      let(:order) { Order.new }
+      let(:order) { build(:order) }
       let(:updater) { order.updater }
       before { allow(order).to receive(:refund_total).and_return(0) }
 
@@ -570,5 +570,31 @@ def create_adjustment(label, amount)
         expect(item).to have_received(:do_something)
       end
     end
+
+    context "with invalid associated objects" do
+      let(:order) { Spree::Order.create(ship_address: Spree::Address.new) }
+
+      before do
+        stub_spree_preferences(run_order_validations_on_order_updater: run_order_validations_on_order_updater)
+      end
+
+      context "when run_order_validations_on_order_updater is true" do
+        let(:run_order_validations_on_order_updater) { true }
+        subject { updater.update }
+
+        it "raises because of the invalid object" do
+          expect { subject }.to raise_error(ActiveRecord::RecordInvalid)
+        end
+      end
+
+      context "when run_order_validations_on_order_updater is false" do
+        let(:run_order_validations_on_order_updater) { false }
+        subject { updater.update }
+
+        it "does not raise because of the invalid object" do
+          expect { subject }.not_to raise_error
+        end
+      end
+    end
   end
 end

core/spec/models/spree/promotion_handler/coupon_spec.rb

@@ -169,7 +169,7 @@ def expect_adjustment_creation(adjustable:, promotion:, promotion_code: nil)
           end
 
           context "applied alongside another valid promotion " do
-            let!(:order) { Order.create }
+            let!(:order) { create(:order) }
 
             before do
               order.coupon_code = "10off"

frontend/app/controllers/spree/orders_controller.rb

@@ -59,12 +59,12 @@ def populate
       # 2,147,483,647 is crazy. See issue https://github.com/spree/spree/issues/2695.
       if !quantity.between?(1, 2_147_483_647)
         @order.errors.add(:base, t('spree.please_enter_reasonable_quantity'))
-      end
-
-      begin
-        @line_item = @order.contents.add(variant, quantity)
-      rescue ActiveRecord::RecordInvalid => error
-        @order.errors.add(:base, error.record.errors.full_messages.join(", "))
+      else
+        begin
+          @line_item = @order.contents.add(variant, quantity)
+        rescue ActiveRecord::RecordInvalid => error
+          @order.errors.add(:base, error.record.errors.full_messages.join(", "))
+        end
       end
 
       respond_with(@order) do |format|