Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

fixed issue 20, STI + deterministic ordering

  • Loading branch information...
commit 69001154984079dbb5204ad3db2b2d2c184f3b25 1 parent 1fc5f1f
@mceachen authored
View
36 README.md
@@ -300,37 +300,40 @@ When you enable ```order```, you'll also have the following new methods injected
If your ```order``` column is an integer attribute, you'll also have these:
-* ```tag.add_sibling_before(sibling_node)``` which will
- 1. move ```tag``` to the same parent as ```sibling_node```,
- 2. decrement the sort_order values of the nodes before the ```sibling_node``` by one, and
- 3. set ```tag```'s order column to 1 less than the ```sibling_node```'s value.
+* ```node1.prepend_sibling(node2)``` which will
+ 1. set ```node2``` to the same parent as ```node1```,
+ 2. set ```node2```'s order column to 1 less than ```node1```'s value, and
+ 3. decrement the order_column of all children of node1's parents whose order_column is <>>= node2's new value by 1.
-* ```tag.add_sibling_after(sibling_node)``` which will
- 1. move ```tag``` to the same parent as ```sibling_node```,
- 2. increment the sort_order values of the nodes after the ```sibling_node``` by one, and
- 3. set ```tag```'s order column to 1 more than the ```sibling_node```'s value.
+* ```node1.append_sibling(node2)``` which will
+ 1. set ```node2``` to the same parent as ```node1```,
+ 2. set ```node2```'s order column to 1 more than ```node1```'s value, and
+ 3. increment the order_column of all children of node1's parents whose order_column is >= node2's new value by 1.
```ruby
+
root = OrderedTag.create(:name => "root")
a = OrderedTag.create(:name => "a", :parent => "root")
b = OrderedTag.create(:name => "b")
c = OrderedTag.create(:name => "c")
+# We have to call 'root.reload.children' because root won't be in sync with the database otherwise:
+
a.append_sibling(b)
-root.children.collect(&:name)
+root.reload.children.collect(&:name)
=> ["a", "b"]
a.prepend_sibling(b)
-root.children.collect(&:name)
+root.reload.children.collect(&:name)
=> ["b", "a"]
a.append_sibling(c)
-root.children.collect(&:name)
-=> ["a", "c", "b"]
+root.reload.children.collect(&:name)
+=> ["b", "a", "c"]
b.append_sibling(c)
-root.children.collect(&:name)
-=> ["a", "b", "c"]
+root.reload.children.collect(&:name)
+=> ["b", "c", "a"]
```
## FAQ
@@ -352,6 +355,11 @@ Closure tree is [tested under every combination](http://travis-ci.org/#!/mceache
## Change log
+### 3.6.1
+
+* Fixed [issue 20](https://github.com/mceachen/closure_tree/issues/20), which affected
+ deterministic ordering when siblings where different STI classes. Thanks, [edwinramirez](https://github.com/edwinramirez)!
+
### 3.6.0
Added support for:
View
19 lib/closure_tree/acts_as_tree.rb
@@ -5,6 +5,7 @@ def acts_as_tree(options = {})
class_attribute :closure_tree_options
self.closure_tree_options = {
+ :ct_base_class => self,
:parent_column_name => 'parent_id',
:dependent => :nullify, # or :destroy or :delete_all -- see the README
:name_column => 'name'
@@ -180,7 +181,7 @@ def descendant_ids
end
def self_and_siblings
- s = self.class.scoped.where(parent_column_sym => parent)
+ s = ct_base_class.where(parent_column_sym => parent)
quoted_order_column ? s.order(quoted_order_column) : s
end
@@ -228,14 +229,14 @@ def find_or_create_by_path(path, attributes = {})
end
def find_all_by_generation(generation_level)
- s = self.class.joins(<<-SQL)
+ s = ct_base_class.joins(<<-SQL)
INNER JOIN (
SELECT descendant_id
FROM #{quoted_hierarchy_table_name}
WHERE ancestor_id = #{self.id}
GROUP BY 1
HAVING MAX(#{quoted_hierarchy_table_name}.generations) = #{generation_level.to_i}
- ) AS descendants ON (#{quoted_table_name}.#{self.class.primary_key} = descendants.descendant_id)
+ ) AS descendants ON (#{quoted_table_name}.#{ct_base_class.primary_key} = descendants.descendant_id)
SQL
order_option ? s.order(order_option) : s
end
@@ -317,7 +318,7 @@ def delete_hierarchy_references
end
def without_self(scope)
- scope.where(["#{quoted_table_name}.#{self.class.primary_key} != ?", self])
+ scope.where(["#{quoted_table_name}.#{ct_base_class.primary_key} != ?", self])
end
def ids_from(scope)
@@ -433,6 +434,11 @@ def ct_class
(self.is_a?(Class) ? self : self.class)
end
+ # This is the "topmost" class. This will only potentially not be ct_class if you are using STI.
+ def ct_base_class
+ ct_class.closure_tree_options[:ct_base_class]
+ end
+
def ct_subclass?
ct_class != ct_class.base_class
end
@@ -516,7 +522,8 @@ def add_sibling(sibling_node, use_update_all = true, add_after = true)
# We need to incr the before_siblings to make room for sibling_node:
if use_update_all
col = quoted_order_column(false)
- ct_class.update_all(
+ # issue 21: we have to use the base class, so STI doesn't get in the way of only updating the child class instances:
+ ct_base_class.update_all(
["#{col} = #{col} #{add_after ? '+' : '-'} 1", "updated_at = now()"],
["#{quoted_parent_column_name} = ? AND #{col} #{add_after ? '>=' : '<='} ?",
ct_parent_id,
@@ -531,7 +538,7 @@ def add_sibling(sibling_node, use_update_all = true, add_after = true)
end
sibling_node.parent = self.parent
sibling_node.save!
- sibling_node.reload
+ sibling_node.reload # <- because siblings_before and siblings_after will have changed.
end
end
end
View
2  lib/closure_tree/version.rb
@@ -1,3 +1,3 @@
module ClosureTree
- VERSION = "3.6.0" unless defined?(::ClosureTree::VERSION)
+ VERSION = "3.6.1" unless defined?(::ClosureTree::VERSION)
end
View
45 spec/label_spec.rb
@@ -115,6 +115,51 @@ def nuke_db
end
end
+ context "deterministically orders with polymorphic siblings" do
+ before :each do
+ @parent = Label.create!(:name => "parent")
+ @a = EventLabel.new(:name => "a")
+ @b = DirectoryLabel.new(:name => "b")
+ @c = DateLabel.new(:name => "c")
+ @parent.children << @a
+ @a.append_sibling(@b)
+ @b.append_sibling(@c)
+ end
+
+ it "when inserted before" do
+ @b.append_sibling(@a)
+ # Have to reload because the sort_order will have changed out from under the references:
+ @b.reload.sort_order.should be < @a.reload.sort_order
+ @a.reload.sort_order.should be < @c.reload.sort_order
+ end
+
+ it "when inserted before" do
+ @b.append_sibling(@a, use_update_all = false)
+ # Have to reload because the sort_order will have changed out from under the references:
+ @b.reload.sort_order.should be < @a.reload.sort_order
+ @a.reload.sort_order.should be < @c.reload.sort_order
+ end
+ end
+
+ it "behaves like the readme" do
+ root = Label.create(:name => "root")
+ a = Label.create(:name => "a", :parent => root)
+ b = Label.create(:name => "b")
+ c = Label.create(:name => "c")
+
+ a.append_sibling(b)
+ root.reload.children.collect(&:name).should == %w(a b)
+
+ a.prepend_sibling(b)
+ root.reload.children.collect(&:name).should == %w(b a)
+
+ a.append_sibling(c)
+ root.reload.children.collect(&:name).should == %w(b a c)
+
+ b.append_sibling(c)
+ root.reload.children.collect(&:name).should == %w(b c a)
+ end
+
context "Deterministic siblings sort with custom integer column" do
nuke_db
fixtures :labels
Please sign in to comment.
Something went wrong with that request. Please try again.