Skip to content

Commit

Permalink
Make sorting more consistent
Browse files Browse the repository at this point in the history
Keep sorting as similar as possible for input and output.
The goal of sorting here is only move parents to in front of children
Or when there is a ranking function, sort the whole tree.

Sorting now keeps closer to the input values
No longer blows away original sort when no order specified

It still has trouble sorting with custom sort method when not all
parent objects are present (it can't sort something not present)
  • Loading branch information
kbrock committed Nov 2, 2018
1 parent bdf39ea commit 432be60
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 22 deletions.
39 changes: 17 additions & 22 deletions lib/ancestry/class_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,12 @@ def orphan_strategy= orphan_strategy

# Arrangement
def arrange options = {}
# Get all nodes ordered by ancestry and start sorting them into an empty hash
arrange_nodes self.ancestry_base_class.reorder(options.delete(:order)).where(options)
# Get all nodes and start sorting them into an empty hash
scope = self.ancestry_base_class
if (order = options.delete(:order))
scope = scope.order(order)
end
arrange_nodes scope.where(options)
end

# Arrange array of nodes into a nested hash of the form
Expand Down Expand Up @@ -61,28 +65,19 @@ def arrange_serializable options={}, nodes=nil, &block

# Pseudo-preordered array of nodes. Children will always follow parents,
# for ordering nodes within a rank provide block, eg. Node.sort_by_ancestry(Node.all) {|a, b| a.rank <=> b.rank}.
# TODO: Fix case when parents are missing and a sort is specified
def sort_by_ancestry(nodes, &block)
arranged = nodes if nodes.is_a?(Hash)

unless arranged
presorted_nodes = nodes.sort do |a, b|
a_cestry, b_cestry = a.ancestry || '0', b.ancestry || '0'

if block_given? && a_cestry == b_cestry
yield a, b
else
a_cestry <=> b_cestry
end
if nodes.is_a?(Hash) || block_given?
nodes = arrange_nodes(nodes) unless nodes.is_a?(Hash)

nodes = nodes.sort { |(a, a_children), (b, b_children)| yield(a, b) } if block_given?
nodes.inject([]) do |sorted_nodes, (node, children)|
sorted_nodes << node
sorted_nodes += sort_by_ancestry(children, &block) unless children.blank?
sorted_nodes
end

arranged = arrange_nodes(presorted_nodes)
end

arranged.inject([]) do |sorted_nodes, pair|
node, children = pair
sorted_nodes << node
sorted_nodes += sort_by_ancestry(children, &block) unless children.blank?
sorted_nodes
else
nodes.sort { |a, b| (b.path_ids - a.path_ids).empty? ? 1 : 0 }
end
end

Expand Down
2 changes: 2 additions & 0 deletions test/concerns/sort_by_ancestry_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ def test_sort_by_ancestry_with_block
assert_equal [n1, n5, n2].map(&:id), model.sort_by_ancestry([n2, n1, n5], &sort).map(&:id)
# does not work for all children no parents
# assert_equal [n5, n3, n4, n6].map(&:id), model.sort_by_ancestry([n3, n4, n5, n6], &sort).map(&:id)
# non ranked works for paginated mid section with missing parents and children works
assert_equal [n3, n2, n4].map(&:id), model.sort_by_ancestry([n3, n2, n4]).map(&:id)
# ranked does not work for ranked case with missing parents
# assert_equal [n3, n2, n4].map(&:id), model.sort_by_ancestry([n3, n2, n4], &sort).map(&:id)
end
Expand Down

0 comments on commit 432be60

Please sign in to comment.