Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't update a child destroyed via relation #261

Merged
merged 8 commits into from Mar 13, 2017
6 changes: 4 additions & 2 deletions lib/acts_as_list/active_record/acts/callback_definer.rb
Expand Up @@ -3,8 +3,10 @@ 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?
# lock! reloads the record and we lose the destroyed_by_association information
# so set_destroyed_by_association_foreign_key must come first
before_destroy :set_destroyed_by_association_foreign_key, :lock!
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?
Expand Down
4 changes: 4 additions & 0 deletions lib/acts_as_list/active_record/acts/list.rb
Expand Up @@ -410,6 +410,10 @@ def position_before_save
end
end

def set_destroyed_by_association_foreign_key
@destroyed_by_association_foreign_key = destroyed_by_association && destroyed_by_association.foreign_key
end

def internal_scope_changed?
return @scope_changed if defined?(@scope_changed)

Expand Down
14 changes: 14 additions & 0 deletions lib/acts_as_list/active_record/acts/scope_method_definer.rb
Expand Up @@ -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 unless @destroyed_by_association_foreign_key
@destroyed_by_association_foreign_key.to_sym == scope
end
elsif scope.is_a?(Array)
define_method :scope_condition do
scope.inject({}) do |hash, column|
Expand All @@ -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 unless @destroyed_by_association_foreign_key
scope_condition.keys.include? @destroyed_by_association_foreign_key
end
else
define_method :scope_condition do
eval "%{#{scope}}"
Expand All @@ -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") }
Expand Down
2 changes: 1 addition & 1 deletion test/test_no_update_for_extra_classes.rb
Expand Up @@ -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

Expand Down
47 changes: 47 additions & 0 deletions test/test_no_update_for_scope_destruction.rb
@@ -0,0 +1,47 @@
require 'helper'

class DestructionTodoList < ActiveRecord::Base
has_many :destruction_todo_items, dependent: :destroy
end

class DestructionTodoItem < ActiveRecord::Base
belongs_to :destruction_todo_list
acts_as_list scope: :destruction_todo_list
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.schema_cache.clear!
[DestructionTodoList, DestructionTodoItem].each(&:reset_column_information)
super
end

def teardown
teardown_db
super
end

class NoUpdateForScopeDestructionTest < NoUpdateForScopeDestructionTestCase
def setup
super
@list = DestructionTodoList.create!

@item_1, @item_2, @item_3 = (1..3).map { |counter| DestructionTodoItem.create!(position: counter, destruction_todo_list_id: @list.id) }
end

def test_no_update_children_when_parent_destroyed
# mock = MiniTest::Mock.new
# mock.expects(:decrement_positions_on_lower_items).once
@list.destroy
end

end
end