Fixed: don't destroy dependent associations if we cannot destroy the Product/Variant

Previously : ensure_no_line_items  callback could stop deleting the
Product/Variant but associations (with dependent: :destroy) were
deleted whatsoever, as this is a rails bug (see
rails/rails#3458)

Author: damianlegawiec

Diff for: core/app/models/spree/product.rb

@@ -26,6 +26,10 @@ class Product < Spree::Base
 
     acts_as_paranoid
 
+    # we need to have this callback before any dependent: :destroy associations
+    # https://github.com/rails/rails/issues/3458
+    before_destroy :ensure_no_line_items
+
     has_many :product_option_types, dependent: :destroy, inverse_of: :product
     has_many :option_types, through: :product_option_types
     has_many :product_properties, dependent: :destroy, inverse_of: :product
@@ -88,7 +92,6 @@ class Product < Spree::Base
     after_save :run_touch_callbacks, if: :anything_changed?
     after_save :reset_nested_changes
     after_touch :touch_taxons
-    before_destroy :ensure_no_line_items
 
     before_validation :normalize_slug, on: :update
     before_validation :validate_master

Diff for: core/app/models/spree/variant.rb

@@ -12,6 +12,10 @@ class Variant < Spree::Base
                         :shipping_category_id, :meta_description, :meta_keywords,
                         :shipping_category
 
+    # we need to have this callback before any dependent: :destroy associations
+    # https://github.com/rails/rails/issues/3458
+    before_destroy :ensure_no_line_items
+
     with_options inverse_of: :variant do
       has_many :inventory_units
       has_many :line_items
@@ -48,7 +52,6 @@ class Variant < Spree::Base
 
     after_create :create_stock_items
     after_create :set_master_out_of_stock, unless: :is_master?
-    before_destroy :ensure_no_line_items
 
     after_touch :clear_in_stock_cache
 

Diff for: core/spec/models/spree/order_merger_spec.rb

@@ -124,7 +124,9 @@ module Spree
       end
 
       it "should create errors with invalid line items" do
-        variant_2.destroy
+        # we cannot use .destroy here as it will be halted by
+        # :ensure_no_line_items callback
+        variant_2.really_destroy!
         subject.merge!(order_2)
         expect(order_1.errors.full_messages).not_to be_empty
       end