Don't update a child destroyed via relation (#261)

Gemfile

@@ -12,6 +12,7 @@ group :test do
   gem "minitest", "~> 5.0"
   gem "test_after_commit", "~> 0.4.2"
   gem "timecop"
+  gem "mocha"
 end
 
 group :sqlite do

gemfiles/rails_3_2.gemfile

@@ -12,6 +12,7 @@ group :test do
   gem "minitest", "~> 5.0"
   gem "test_after_commit", "~> 0.4.2"
   gem "timecop"
+  gem "mocha"
   gem "after_commit_exception_notification"
 end
 

gemfiles/rails_4_1.gemfile

@@ -12,6 +12,7 @@ group :test do
   gem "minitest", "~> 5.0"
   gem "test_after_commit", "~> 0.4.2"
   gem "timecop"
+  gem "mocha"
   gem "after_commit_exception_notification"
 end
 

gemfiles/rails_4_2.gemfile

@@ -12,6 +12,7 @@ group :test do
   gem "minitest", "~> 5.0"
   gem "test_after_commit", "~> 0.4.2"
   gem "timecop"
+  gem "mocha"
 end
 
 group :sqlite do

gemfiles/rails_5_0.gemfile

@@ -12,6 +12,7 @@ group :test do
   gem "minitest", "~> 5.0"
   gem "test_after_commit", "~> 0.4.2"
   gem "timecop"
+  gem "mocha"
 end
 
 group :sqlite do

gemfiles/rails_5_1.gemfile

@@ -12,6 +12,7 @@ group :test do
   gem "minitest", "~> 5.0"
   gem "test_after_commit", "~> 0.4.2"
   gem "timecop"
+  gem "mocha"
 end
 
 group :sqlite do

lib/acts_as_list/active_record/acts/callback_definer.rb

@@ -3,8 +3,8 @@ def self.call(caller_class, add_new_at)
     caller_class.class_eval do
       before_validation :check_top_position, unless: :act_as_list_no_update?
 
-      before_destroy :lock!
-      after_destroy :decrement_positions_on_lower_items, unless: :act_as_list_no_update?
+      before_destroy :lock!, unless: "destroyed_via_scope? || act_as_list_no_update?"
+      after_destroy :decrement_positions_on_lower_items, unless: "destroyed_via_scope? || act_as_list_no_update?"
 
       before_update :check_scope, unless: :act_as_list_no_update?
       after_update :update_positions, unless: :act_as_list_no_update?

lib/acts_as_list/active_record/acts/scope_method_definer.rb

@@ -17,6 +17,11 @@ def self.call(caller_class, scope)
         define_method :scope_changed? do
           changed.include?(scope_name.to_s)
         end
+
+        define_method :destroyed_via_scope? do
+          return false if ActiveRecord::VERSION::MAJOR < 4
+          scope == (destroyed_by_association && destroyed_by_association.foreign_key.to_sym)
+        end
       elsif scope.is_a?(Array)
         define_method :scope_condition do
           scope.inject({}) do |hash, column|
@@ -27,6 +32,11 @@ def self.call(caller_class, scope)
         define_method :scope_changed? do
           (scope_condition.keys & changed.map(&:to_sym)).any?
         end
+
+        define_method :destroyed_via_scope? do
+          return false if ActiveRecord::VERSION::MAJOR < 4
+          scope_condition.keys.include? (destroyed_by_association && destroyed_by_association.foreign_key.to_sym)
+        end
       else
         define_method :scope_condition do
           eval "%{#{scope}}"
@@ -35,6 +45,10 @@ def self.call(caller_class, scope)
         define_method :scope_changed? do
           false
         end
+
+        define_method :destroyed_via_scope? do
+          false
+        end
       end
 
       self.scope :in_list, lambda { where("#{quoted_position_column_with_table_name} IS NOT NULL") }

test/helper.rb

@@ -11,6 +11,7 @@
 end
 require "active_record"
 require "minitest/autorun"
+require "mocha/mini_test"
 require "#{File.dirname(__FILE__)}/../init"
 
 if defined?(ActiveRecord::VERSION) &&

test/test_no_update_for_extra_classes.rb

@@ -33,7 +33,7 @@ def setup
     end
 
     ActiveRecord::Base.connection.schema_cache.clear!
-    [TodoList, Item, TodoItemAttachment].each(&:reset_column_information)
+    [TodoList, TodoItem, TodoItemAttachment].each(&:reset_column_information)
     super
   end
 

test/test_no_update_for_scope_destruction.rb

@@ -0,0 +1,72 @@
+require 'helper'
+
+class DestructionTodoList < ActiveRecord::Base
+  has_many :destruction_todo_items, dependent: :destroy
+  has_many :destruction_tada_items, dependent: :destroy
+end
+
+class DestructionTodoItem < ActiveRecord::Base
+  belongs_to :destruction_todo_list
+  acts_as_list scope: :destruction_todo_list
+end
+
+class DestructionTadaItem < ActiveRecord::Base
+  belongs_to :destruction_todo_list
+  acts_as_list scope: [:destruction_todo_list_id, :enabled]
+end
+
+class NoUpdateForScopeDestructionTestCase < Minitest::Test
+  def setup
+    ActiveRecord::Base.connection.create_table :destruction_todo_lists do |t|
+    end
+
+    ActiveRecord::Base.connection.create_table :destruction_todo_items do |t|
+      t.column :position, :integer
+      t.column :destruction_todo_list_id, :integer
+    end
+
+    ActiveRecord::Base.connection.create_table :destruction_tada_items do |t|
+      t.column :position, :integer
+      t.column :destruction_todo_list_id, :integer
+      t.column :enabled, :boolean
+    end
+
+    ActiveRecord::Base.connection.schema_cache.clear!
+    [DestructionTodoList, DestructionTodoItem, DestructionTadaItem].each(&:reset_column_information)
+    super
+  end
+
+  def teardown
+    teardown_db
+    super
+  end
+
+  class NoUpdateForScopeDestructionTest < NoUpdateForScopeDestructionTestCase
+    def setup
+      super
+      @list = DestructionTodoList.create!
+
+      @todo_item_1 = DestructionTodoItem.create! position: 1, destruction_todo_list_id: @list.id
+      @tada_item_1 = DestructionTadaItem.create! position: 1, destruction_todo_list_id: @list.id, enabled: true
+    end
+
+    def test_no_update_children_when_parent_destroyed
+      if ActiveRecord::VERSION::MAJOR < 4
+        DestructionTodoItem.any_instance.expects(:decrement_positions_on_lower_items).once
+        DestructionTadaItem.any_instance.expects(:decrement_positions_on_lower_items).once
+      else
+        DestructionTodoItem.any_instance.expects(:decrement_positions_on_lower_items).never
+        DestructionTadaItem.any_instance.expects(:decrement_positions_on_lower_items).never
+      end
+      assert @list.destroy
+    end
+
+    def test_update_children_when_sibling_destroyed
+      @todo_item_1.expects(:decrement_positions_on_lower_items).once
+      @tada_item_1.expects(:decrement_positions_on_lower_items).once
+      assert @todo_item_1.destroy
+      assert @tada_item_1.destroy
+    end
+
+  end
+end