Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit 29c06c6

Browse files
author
Tom Smyth
committed
Added dont_order_roots option
1 parent 6fcfe12 commit 29c06c6

File tree

9 files changed

+127
-28
lines changed

9 files changed

+127
-28
lines changed

‎README.md‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -489,6 +489,25 @@ root.reload.children.pluck(:name)
489489
=> ["b", "c", "a"]
490490
```
491491

492+
### Ordering Roots
493+
494+
With numeric ordering, root nodes are, by default, assigned order values globally across the whole database
495+
table. So for instance if you have 5 nodes with no parent, they will be ordered 0 through 4 by default.
496+
If your model represents many separate trees and you have a lot of records, this can cause performance
497+
problems, and doesn't really make much sense.
498+
499+
You can disable this default behavior by passing `dont_order_roots: true` as an option to your delcaration:
500+
501+
```
502+
has_closure_tree order: 'sort_order', numeric_order: true, dont_order_roots: true
503+
```
504+
505+
In this case, calling `prepend_sibling` and `append_sibling` on a root node or calling
506+
`roots_and_descendants_preordered` on the model will raise a `RootOrderingDisabledError`.
507+
508+
The `dont_order_roots` option will be ignored unless `numeric_order` is set to true.
509+
510+
492511
## Concurrency
493512
494513
Several methods, especially ```#rebuild``` and ```#find_or_create_by_path```, cannot run concurrently correctly.

‎lib/closure_tree/has_closure_tree.rb‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ def has_closure_tree(options = {})
88
:hierarchy_table_name,
99
:name_column,
1010
:order,
11+
:dont_order_roots,
1112
:numeric_order,
1213
:touch,
1314
:with_advisory_lock

‎lib/closure_tree/has_closure_tree_root.rb‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
module ClosureTree
22
class MultipleRootError < StandardError; end
3+
class RootOrderingDisabledError < StandardError; end
34

45
module HasClosureTreeRoot
56

‎lib/closure_tree/numeric_deterministic_ordering.rb‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,15 @@ def _ct_sum_order_by(node = nil)
6565
node_score = "(1 + anc.#{_ct.quoted_order_column(false)}) * " +
6666
"power(#{h['total_descendants']}, #{h['max_depth'].to_i + 1} - #{depth_column})"
6767

68-
Arel.sql("SUM(#{node_score})")
68+
# We want the NULLs to be first in case we are not ordering roots and they have NULL order.
69+
Arel.sql("SUM(#{node_score}) IS NULL DESC, SUM(#{node_score})")
6970
end
7071

7172
def roots_and_descendants_preordered
73+
if _ct.dont_order_roots
74+
raise ClosureTree::RootOrderingDisabledError.new("Root ordering is disabled on this model")
75+
end
76+
7277
join_sql = <<-SQL.strip_heredoc
7378
JOIN #{_ct.quoted_hierarchy_table_name} anc_hier
7479
ON anc_hier.descendant_id = #{_ct.quoted_table_name}.#{_ct.quoted_id_column_name}
@@ -113,6 +118,10 @@ def prepend_sibling(sibling_node)
113118
def add_sibling(sibling, add_after = true)
114119
fail "can't add self as sibling" if self == sibling
115120

121+
if _ct.dont_order_roots && parent.nil?
122+
raise ClosureTree::RootOrderingDisabledError.new("Root ordering is disabled on this model")
123+
end
124+
116125
# Make sure self isn't dirty, because we're going to call reload:
117126
save
118127

‎lib/closure_tree/numeric_order_support.rb‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ def self.adapter_for_connection(connection)
1414

1515
module MysqlAdapter
1616
def reorder_with_parent_id(parent_id, minimum_sort_order_value = nil)
17+
return if parent_id.nil? && dont_order_roots
1718
min_where = if minimum_sort_order_value
1819
"AND #{quoted_order_column} >= #{minimum_sort_order_value}"
1920
else
@@ -31,6 +32,7 @@ def reorder_with_parent_id(parent_id, minimum_sort_order_value = nil)
3132

3233
module PostgreSQLAdapter
3334
def reorder_with_parent_id(parent_id, minimum_sort_order_value = nil)
35+
return if parent_id.nil? && dont_order_roots
3436
min_where = if minimum_sort_order_value
3537
"AND #{quoted_order_column} >= #{minimum_sort_order_value}"
3638
else
@@ -56,6 +58,7 @@ def rows_updated(result)
5658

5759
module GenericAdapter
5860
def reorder_with_parent_id(parent_id, minimum_sort_order_value = nil)
61+
return if parent_id.nil? && dont_order_roots
5962
scope = model_class.
6063
where(parent_column_sym => parent_id).
6164
order(nulls_last_order_by)

‎lib/closure_tree/support_attributes.rb‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,10 @@ def order_by
7575
options[:order]
7676
end
7777

78+
def dont_order_roots
79+
options[:dont_order_roots] || false
80+
end
81+
7882
def nulls_last_order_by
7983
"-#{quoted_order_column} #{order_by_order(reverse = true)}"
8084
end

‎spec/db/database.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ sqlite:
1313
postgresql:
1414
<<: *common
1515
adapter: postgresql
16-
username: postgres
16+
username: tomsmyth
1717

1818
mysql:
1919
<<: *common

‎spec/db/models.rb‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,21 @@ class DateLabel < Label
9999
class DirectoryLabel < Label
100100
end
101101

102+
class LabelWithoutRootOrdering < ActiveRecord::Base
103+
# make sure order doesn't matter
104+
acts_as_tree :order => :column_whereby_ordering_is_inferred, # <- symbol, and not "sort_order"
105+
:numeric_order => true,
106+
:dont_order_roots => true,
107+
:parent_column_name => "mother_id",
108+
:hierarchy_table_name => "label_hierarchies"
109+
110+
self.table_name = "#{table_name_prefix}labels#{table_name_suffix}"
111+
112+
def to_s
113+
"#{self.class}: #{name}"
114+
end
115+
end
116+
102117
class CuisineType < ActiveRecord::Base
103118
acts_as_tree
104119
end

‎spec/label_spec.rb‎

Lines changed: 73 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,50 @@ def roots_name_and_order
289289
end
290290
end
291291

292+
context "doesn't order roots when requested" do
293+
before :each do
294+
@root1 = LabelWithoutRootOrdering.create!(:name => 'root1')
295+
@root2 = LabelWithoutRootOrdering.create!(:name => 'root2')
296+
@a, @b, @c, @d, @e = ('a'..'e').map { |ea| LabelWithoutRootOrdering.new(:name => ea) }
297+
@root1.children << @a
298+
@root1.append_child(@c)
299+
@root1.prepend_child(@d)
300+
301+
# Reload is needed here and below because order values may have been adjusted in the DB during
302+
# prepend_child, append_sibling, etc.
303+
[@a, @c, @d].each(&:reload)
304+
305+
@a.append_sibling(@b)
306+
[@a, @c, @d, @b].each(&:reload)
307+
@d.prepend_sibling(@e)
308+
end
309+
310+
it 'order_values properly' do
311+
expect(@root1.reload.order_value).to be_nil
312+
orders_and_names = @root1.children.reload.map { |ea| [ea.name, ea.order_value] }
313+
expect(orders_and_names).to eq([['e', 0], ['d', 1], ['a', 2], ['b', 3], ['c', 4]])
314+
end
315+
316+
it 'raises on prepending and appending to root' do
317+
expect { @root1.prepend_sibling(@f) }.to raise_error(ClosureTree::RootOrderingDisabledError)
318+
expect { @root1.append_sibling(@f) }.to raise_error(ClosureTree::RootOrderingDisabledError)
319+
end
320+
321+
it 'returns empty array for siblings_before and after' do
322+
expect(@root1.siblings_before).to eq([])
323+
expect(@root1.siblings_after).to eq([])
324+
end
325+
326+
it 'returns expected result for self_and_descendants_preordered' do
327+
expect(@root1.self_and_descendants_preordered.to_a).to eq([@root1, @e, @d, @a, @b, @c])
328+
end unless sqlite? # sqlite doesn't have a power function.
329+
330+
it 'raises on roots_and_descendants_preordered' do
331+
expect { LabelWithoutRootOrdering.roots_and_descendants_preordered }.to raise_error(
332+
ClosureTree::RootOrderingDisabledError)
333+
end
334+
end
335+
292336
describe 'code in the readme' do
293337
it 'creates STI label hierarchies' do
294338
child = Label.find_or_create_by_path([
@@ -341,17 +385,7 @@ def roots_name_and_order
341385
root = Label.create(:name => "root")
342386
a = Label.create(:name => "a", :parent => root)
343387
b = Label.create(:name => "b", :parent => root)
344-
expect(a.order_value).to eq(0)
345-
expect(b.order_value).to eq(1)
346-
#c = Label.create(:name => "c")
347388

348-
# should the order_value for roots be set?
349-
expect(root.order_value).not_to be_nil
350-
expect(root.order_value).to eq(0)
351-
352-
# order_value should never be nil on a child.
353-
expect(a.order_value).not_to be_nil
354-
expect(a.order_value).to eq(0)
355389
# Add a child to root at end of children.
356390
root.children << b
357391
expect(b.parent).to eq(root)
@@ -395,28 +429,41 @@ def roots_name_and_order
395429
end
396430

397431
context "order_value must be set" do
432+
shared_examples_for "correct order_value" do
433+
before do
434+
@root = model.create(name: 'root')
435+
@a, @b, @c = %w(a b c).map { |n| @root.children.create(name: n) }
436+
end
398437

399-
before do
400-
@root = Label.create(name: 'root')
401-
@a, @b, @c = %w(a b c).map { |n| @root.children.create(name: n) }
402-
end
438+
it 'should set order_value on roots' do
439+
expect(@root.order_value).to eq(expected_root_order_value)
440+
end
403441

404-
it 'should set order_value on roots' do
405-
expect(@root.order_value).to eq(0)
442+
it 'should set order_value with siblings' do
443+
expect(@a.order_value).to eq(0)
444+
expect(@b.order_value).to eq(1)
445+
expect(@c.order_value).to eq(2)
446+
end
447+
448+
it 'should reset order_value when a node is moved to another location' do
449+
root2 = model.create(name: 'root2')
450+
root2.add_child @b
451+
expect(@a.order_value).to eq(0)
452+
expect(@b.order_value).to eq(0)
453+
expect(@c.reload.order_value).to eq(1)
454+
end
406455
end
407456

408-
it'should set order_value with siblings' do
409-
expect(@a.order_value).toeq(0)
410-
expect(@b.order_value).toeq(1)
411-
expect(@c.order_value).toeq(2)
457+
context"with normal model" do
458+
let(:model){Label}
459+
let(:expected_root_order_value){0}
460+
it_behaves_like"correct order_value"
412461
end
413462

414-
it 'should reset order_value when a node is moved to another location' do
415-
root2 = Label.create(name: 'root2')
416-
root2.add_child @b
417-
expect(@a.order_value).to eq(0)
418-
expect(@b.order_value).to eq(0)
419-
expect(@c.reload.order_value).to eq(1)
463+
context "without root ordering" do
464+
let(:model) { LabelWithoutRootOrdering }
465+
let(:expected_root_order_value) { nil }
466+
it_behaves_like "correct order_value"
420467
end
421468
end
422469

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /