Negative position shows child before parent in roots_and_descendants_preordered

[leonelgalan]

When position is negative the child showed before the parent in roots_and_descendants_preordered, self_and_descendants_preordered it's behaving correctly (displaying Parent first, followed by its children)

  Child A
Parent
  Child B
  Child C

[mceachen]

Negative values aren't supported, but pull requests are. ;)

(I think this would be pretty tricky/impossible to support, if you
look at the equation we use. If you use any of the pretend/append
sibling methods, you wouldn't have negative values, so you set those
specifically to a negative value? If you want to fix your current db,
it'd be pretty simple to run through all nodes and give them new
positive values).

[elhoyos]

If you use any of the pretend/append sibling methods, you wouldn't have negative values...

Are we sure on this? My experience is that add_sibling will give you negative values on the sort order column for all previous siblings when inserting into, but prepending/appending to the first/last sibling will not.

[mceachen]

@elhoyos you're completely correct, sorry about that:

irb(main):002:0> l = Label.create(:name => "one")
=> #<Label id: 1, name: "one", type: nil, sort_order: nil, mother_id: nil>
irb(main):003:0> l.prepend_sibling(Label.new(:name => "zero"))
=> #<Label id: 2, name: "zero", type: nil, sort_order: -1, mother_id: nil>

So—there's a couple things we can do.

A) make the append_sibling/prepend_sibling functions ensure there are only positive order values that have no numeric gaps

B) make roots_and_descendants_preordered handle negative values and non-consecutive order values

Which sounds better?

[elhoyos]

A) sounds cleaner and thus better.

Juan David

On Wed, Mar 20, 2013 at 11:24 PM, Matthew McEachen <[email protected]

wrote:

@elhoyos https://github.com/elhoyos you're completely correct, sorry
about that.

So—there's a couple things we can do.

A) make the append_sibling/prepend_sibling functions ensure there are only
positive order values that have no numeric gaps

B) make roots_and_descendants_preordered handle negative values and
non-consecutive order values

Which sounds better?

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/49#issuecomment-15218953
.

[elfassy]

B) sounds better for people that want to use ranked-model to generate the position

https://github.com/mixonic/ranked-model

[mceachen]

Just looked at rank_model, and it would also require a synthetic
row_number calculation.

[elfassy]

"node1.append_sibling(node2) which will (...) increment the order_column of all children of node1's parents whose order_column is >= node2's new value by 1."

Needless to say, that's not very efficient.

How about letting people implement their own ordering. With ranked_model it would give something like:

acts_as_tree :order => 'position'
include RankedModel
ranks :position, :with_same => :parent_id

[brendon]

I'm in the process of switching from ancestry to closure_tree because of the bad interaction between acts_as_list and ancestry. They step on each others toes when deleting a branch of the tree because acts_as_list wants to reshuffle positions on surrounding nodes as they're deleted even though all the nodes eventually get deleted. This results in AR not found errors.

I'd imagine this would be the case for any approach that had a separate ordering system as at the very least the tree system needs to make sure it destroys nodes starting from the leaves upward. The best case would be that the tree system knows about position and turns off compensation for removing nodes that it knows will be destroyed anyway.

I'm not sure of the details of ranked_model but it appears to work in the same way as acts_as_list except for the consecutive position values.

[brendon]

Just a little bit of followup. Would these be better scenarios for add_child, prepend_sibling, and append_sibling when using numeric deterministic ordering?

ref_node.add_child(node)

• Remove node from existing parent (if node exists in the tree already)
• Update position of node's siblings to be sibling.position - 1 where sibling.position > node.original_position (if node exists in the tree already)
• Insert node as child of ref_node
• Set position of node to be last child of ref_node (excluding node) + 1

ref_node.prepend_sibling(node)

• Remove node from existing parent (if node exists in the tree already)
• Update position of node's siblings to be sibling.position - 1 where sibling.position > node.original_position (if node exists in the tree already)
• Update position of ref_node's siblings to be sibling.position + 1 where sibling.position >= ref_node's position
• Insert node as a child of ref_node's parent with the original position of ref_node

ref_node.append_sibling(node)

• Remove node from existing parent (if node exists in the tree already)
• Update position of node's siblings to be sibling.position - 1 where sibling.position > node.original_position (if node exists in the tree already)
• Update position of ref_node's siblings to be sibling.position + 1 where sibling.position > ref_node's position
• Insert node as a child of ref_node's parent with the position of ref_node + 1

I would have created some tests and a pull request but I had trouble getting a test suite set up. :)

[mceachen]

This is released in 4.0.1.

[leomayleomay]

This is still not working quite well with RankedModel, it works great before I adjust the position of an item, it breaks miserably after I adjust the position, and the new row_order became a negative number, I am not appending/prepending a child/sibling, I am just re-arranging the position.

[leomayleomay]

finally, i figured this part out with overwriting the method in the model, it's slow (because it's recursive), but it works

def self_and_descendents_preordered
  result = [self]

  children.each do |child|
    result += child.self_and_descendents_preordered
  end

  result
end