Skip to content

Commit

Permalink
Reorder callbacks should not be triggered if none of tree attributes …
Browse files Browse the repository at this point in the history
…were changed.

Callbacks specs also moved to separate file.
  • Loading branch information
take-five committed Feb 19, 2014
1 parent 94b2647 commit e40486a
Show file tree
Hide file tree
Showing 5 changed files with 176 additions and 270 deletions.
2 changes: 1 addition & 1 deletion lib/acts_as_ordered_tree/position.rb
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ def decrement_counter
# @param [ActsAsOrderedTree::Node::Position] other
def ==(other)
other.is_a?(self.class) &&
other.node == node &&
other.record == record &&
other.parent_id == parent_id &&
other.position == position
end
Expand Down
66 changes: 0 additions & 66 deletions spec/acts_as_ordered_tree_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -232,16 +232,6 @@
let!(:child_3) { create :default_with_counter_cache, :parent => root, :name => 'child_3' }
let!(:child_4) { create :default_with_counter_cache, :parent => child_3, :name => 'child_4' }

context "initial" do
specify { expect([child_1, child_2, child_3]).to be_sorted }

subject { root.reload }
its(:parent_id) { should be_nil }
its(:level) { should be_zero }
its(:position) { should eq 1 }
its(:categories_count) { should eq 3}
end

describe "#insert_at" do
before { ActiveSupport::Deprecation.silence { child_3.insert_at(1) } }
before { child_3.reload }
Expand All @@ -252,31 +242,6 @@
describe "callbacks" do
subject { child_3 }

it { should fire_callback(:before_move).when_calling(:move_to_root).once }
it { should fire_callback(:after_move).when_calling(:move_to_root).once }
it { should fire_callback(:around_move).when_calling(:move_to_root).once }

it { should_not fire_callback(:before_move).when_calling(:move_left) }
it { should_not fire_callback(:after_move).when_calling(:move_left) }
it { should_not fire_callback(:around_move).when_calling(:move_left) }

it { should fire_callback(:before_reorder).when_calling(:move_higher).once }
it { should fire_callback(:after_reorder).when_calling(:move_higher).once }

it { should_not fire_callback(:before_reorder).when_calling(:move_to_root) }

it "should cache depth on save" do
record = build :default_with_counter_cache

record.depth.should be_nil
record.save

record.depth.should eq 0

record.move_to_left_of child_3
record.depth.should eq child_3.level
end

it "should recalculate depth of descendants" do
record = create :default_with_counter_cache, :parent => child_3
record.depth.should eq 2
Expand All @@ -297,24 +262,6 @@

record.reload.depth.should eq 3
end

context "DefaultWithCallbacks" do
let!(:cb_root_1) { create :default_with_callbacks, :name => 'root_1' }
let!(:cb_root_2) { create :default_with_callbacks, :name => 'root_2' }
let!(:cb_child_1) { create :default_with_callbacks, :name => 'child_1', :parent => cb_root_1 }
let!(:cb_child_2) { create :default_with_callbacks, :name => 'child_2', :parent => cb_root_1 }

specify "new parent_id should be available in before_move" do
cb_root_2.stub(:before_move) { cb_root_2.parent_id.should eq cb_root_1.id }
cb_root_2.move_to_left_of cb_child_1
end

specify "new position should be available in before_reorder" do
cb_child_2.stub(:before_reorder) { cb_child_2.position.should eq 1 }
cb_child_2.move_to_left_of cb_child_1
end
end

end

context "changed attributes" do
Expand Down Expand Up @@ -389,18 +336,5 @@
it { should have_at_least(1).error_on(:parent) }
end
end

describe "attempt to create node with wrong position" do
it "should not throw error" do
expect{ create :default, :position => 22 }.not_to raise_error
end

it "should be saved at proper position" do
root = create :default

node = create :default, :position => 2
node.position.should eq 2
end
end
end
end
175 changes: 175 additions & 0 deletions spec/callbacks_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
# coding: utf-8

require 'spec_helper'

describe ActsAsOrderedTree, 'before/after add/remove callbacks', :transactional do
class Class1 < Default
cattr_accessor :triggered_callbacks

def self.triggered?(kind, *args)
if args.any?
triggered_callbacks.include?([kind, *args])
else
triggered_callbacks.any? { |c| c.first == kind }
end
end

acts_as_ordered_tree :before_add => :before_add,
:after_add => :after_add,
:before_remove => :before_remove,
:after_remove => :after_remove

before_reorder :before_reorder
after_reorder :after_reorder
before_move :before_move
after_move :after_move

def run_callback(kind, *args)
self.class.triggered_callbacks ||= []
self.class.triggered_callbacks << [kind, self, *args]
end

CALLBACKS = [:before, :after].product([:add, :remove, :move, :reorder]).map { |a, b| "#{a}_#{b}".to_sym }

CALLBACKS.each do |callback|
define_method(callback) { |*args| run_callback(callback, *args) }
end
end

matcher :trigger_callback do |*callbacks, &block|
match_for_should do |proc|
Class1.triggered_callbacks = []
proc.call
callbacks.all? { |callback| Class1.triggered?(callback, *@with) }
end

match_for_should_not do |proc|
Class1.triggered_callbacks = []
proc.call
callbacks.none? { |callback| Class1.triggered?(callback, *@with) }
end

chain :with do |*args, &block|
@with = args
@block = block
end

description do
description = "trigger callbacks #{callbacks.map(&:inspect).join(', ')}"
description << " with arguments #{@with.inspect}" if @with
description
end

failure_message_for_should do
"expected that block would #{description}"
end

failure_message_for_should_not do
desc = "expected that block would not #{description}, but following callbacks were triggered:"
Class1.triggered_callbacks.each do |kind, *args|
desc << "\n * #{kind.inspect} #{args.inspect}"
end
desc
end
end
alias_method :trigger_callbacks, :trigger_callback

# root
# child 1
# child 2
# child 3
# child 4
# child 5
let(:root) { Class1.create :name => 'root' }
let!(:child1) { Class1.create :name => 'child1', :parent => root }
let!(:child2) { Class1.create :name => 'child2', :parent => child1 }
let!(:child3) { Class1.create :name => 'child3', :parent => child1 }
let!(:child4) { Class1.create :name => 'child4', :parent => root }
let!(:child5) { Class1.create :name => 'child5', :parent => child4 }

it 'does not trigger any callbacks when tree attributes were not changed' do
expect {
child2.update_attributes :name => 'x'
}.not_to trigger_callbacks(*Class1::CALLBACKS)
end

it 'does not trigger any callbacks except :before_remove and :after_remove when node is destroyed' do
expect {
child2.destroy
}.not_to trigger_callbacks(*Class1::CALLBACKS - [:before_remove, :after_remove])
end

describe '*_add callbacks' do
let(:new_record) { Class1.new :name => 'child6' }

it 'fires *_add callbacks when new children added to node' do
expect {
child1.children << new_record
}.to trigger_callbacks(:before_add, :after_add).with(child1, new_record)
end

it 'fires *_add callbacks when node is moved from another parent' do
expect {
child2.update_attributes!(:parent => child4)
}.to trigger_callbacks(:before_add, :after_add).with(child4, child2)
end
end

describe '*_remove callbacks' do
it 'fires *_remove callbacks when node is moved from another parent' do
expect {
child2.update_attributes!(:parent => child4)
}.to trigger_callbacks(:before_remove, :after_remove).with(child1, child2)
end

it 'fires *_remove callbacks when node is destroyed' do
expect {
child2.destroy
}.to trigger_callbacks(:before_remove, :after_remove).with(child1, child2)
end
end

describe '*_move callbacks' do
it 'fires *_move callbacks when node is moved to another parent' do
expect {
child2.update_attributes!(:parent => child4)
}.to trigger_callbacks(:before_move, :after_move).with(child2)
end

it 'does not trigger *_move callbacks when node is not moved to another parent' do
expect {
child2.move_lower
}.not_to trigger_callbacks(:before_move, :after_move)

expect {
root.move_to_root
}.not_to trigger_callbacks(:before_move, :after_move)
end
end

describe '*_reorder callbacks' do
it 'fires *_reorder callbacks when node position is changed but parent not' do
expect {
child2.move_lower
}.to trigger_callbacks(:before_reorder, :after_reorder).with(child2)
end

it 'does not fire *_reorder callbacks when node is moved to another parent' do
expect {
child2.move_to_root
}.not_to trigger_callbacks(:before_reorder, :after_reorder)
end
end

describe 'Callbacks context' do
specify 'new parent_id should be available in before_move' do
expect(child2).to receive(:before_move) { expect(child2.parent_id).to eq child4.id }
child2.update_attributes! :parent => child4
end

specify 'new position should be available in before_reorder' do
expect(child2).to receive(:before_reorder) { expect(child2.position).to eq 2 }
child2.move_lower
end
end
end
93 changes: 0 additions & 93 deletions spec/move_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -86,97 +86,4 @@ def move(node, new_parent, new_position)

it { expect(root.position).to eq 2 }
end
end

describe ActsAsOrderedTree, 'before/after add/remove callbacks', :transactional do
class Class1 < Default
cattr_accessor :triggered_callbacks

def self.triggered?(kind, *args)
triggered_callbacks.include?([kind, *args])
end

acts_as_ordered_tree :before_add => :before_add,
:after_add => :after_add,
:before_remove => :before_remove,
:after_remove => :after_remove

def run_callback(kind, arg)
self.class.triggered_callbacks ||= []
self.class.triggered_callbacks << [kind, self, arg]
end

def before_remove(record)
run_callback(:before_remove, record)
end

def after_remove(record)
run_callback(:after_remove, record)
end

def before_add(record)
run_callback(:before_add, record)
end

def after_add(record)
run_callback(:after_add, record)
end
end

# root
# child 1
# child 2
# child 3
# child 4
# child 5
let(:root) { Class1.create :name => 'root' }
let!(:child1) { Class1.create :name => 'child1', :parent => root }
let!(:child2) { Class1.create :name => 'child2', :parent => child1 }
let!(:child3) { Class1.create :name => 'child3', :parent => child1 }
let!(:child4) { Class1.create :name => 'child4', :parent => root }
let!(:child5) { Class1.create :name => 'child5', :parent => child4 }

def test_callback(record, kind, expected_arg, &block)
Class1.triggered_callbacks = []

expect(&block).to change{Class1.triggered?(kind, record, expected_arg)}.to(true)
end

describe '*_add callbacks' do
let(:new_record) { Class1.new :name => 'child6' }

it 'fires before_add callback when new children added to node' do
test_callback(child1, :before_add, new_record) { child1.children << new_record }
end

it 'fires after_add callback when new children added to node' do
test_callback(child1, :after_add, new_record) { child1.children << new_record }
end

it 'fires before_add callback when node is moved from another parent' do
test_callback(child4, :before_add, child2) { child2.update_attributes!(:parent => child4) }
end

it 'fires after_add callback when node is moved from another parent' do
test_callback(child4, :after_add, child2) { child2.update_attributes!(:parent => child4) }
end
end

describe '*_remove callbacks' do
it 'fires before_remove callback when node is moved from another parent' do
test_callback(child1, :before_remove, child2) { child2.update_attributes!(:parent => child4) }
end

it 'fires after_remove callback when node is moved from another parent' do
test_callback(child1, :after_remove, child2) { child2.update_attributes!(:parent => child4) }
end

it 'fires before_remove callback when node is destroyed' do
test_callback(child1, :before_remove, child2) { child2.destroy }
end

it 'fires after_remove callback when node is destroyed' do
test_callback(child1, :after_remove, child2) { child2.destroy }
end
end
end
Loading

0 comments on commit e40486a

Please sign in to comment.