Merge pull request spree#7430 from vishalzambre/user_before_destroy

Fixed: don't destroy dependent associations if we cannot destroy the User

core/app/models/concerns/spree/user_methods.rb

@@ -8,6 +8,10 @@ module UserMethods
     include Spree::RansackableAttributes
 
     included do
+      # we need to have this callback before any dependent: :destroy associations
+      # https://github.com/rails/rails/issues/3458
+      before_destroy :check_completed_orders
+
       has_many :role_users, class_name: 'Spree::RoleUser', foreign_key: :user_id, dependent: :destroy
       has_many :spree_roles, through: :role_users, class_name: 'Spree::Role', source: :role
 
@@ -43,5 +47,11 @@ def analytics_id
     def total_available_store_credit
       store_credits.reload.to_a.sum(&:amount_remaining)
     end
+
+    private
+
+    def check_completed_orders
+      raise Spree::Core::DestroyWithOrdersError if orders.complete.present?
+    end
   end
 end

core/app/models/spree/legacy_user.rb

@@ -7,15 +7,7 @@ class LegacyUser < Spree::Base
 
     self.table_name = 'spree_users'
 
-    before_destroy :check_completed_orders
-
     attr_accessor :password
     attr_accessor :password_confirmation
-
-    private
-
-      def check_completed_orders
-        raise Spree::Core::DestroyWithOrdersError if orders.complete.present?
-      end
   end
 end

core/spec/models/spree/concerns/user_methods_spec.rb

@@ -52,4 +52,31 @@
       it { is_expected.to be_nil }
     end
   end
+
+  context '#check_completed_orders' do
+    let(:possible_promotion) { create(:promotion, advertise: true, starts_at: 1.day.ago) }
+    context 'rstrict t delete dependent destroyed' do
+      before do
+        test_user.promotion_rules.create!(promotion: possible_promotion)
+        create(:order, user: test_user, completed_at: Time.current)
+      end
+
+      it 'should not destroy dependent destroy items' do
+        expect { test_user.destroy }.to raise_error(Spree::Core::DestroyWithOrdersError)
+        expect(test_user.promotion_rule_users.any?).to be(true)
+      end
+    end
+
+    context 'allow to destroy dependent destroy' do
+      before do
+        test_user.promotion_rules.create!(promotion: possible_promotion)
+        create(:order, user: test_user, created_at: 1.day.ago)
+        test_user.destroy
+      end
+
+      it 'should not destroy dependent destroy items' do
+        expect(test_user.promotion_rule_users.any?).to be(false)
+      end
+    end
+  end
 end