ClosureTree / closure_tree

Easily and efficiently make your ActiveRecord models support hierarchies
https://closuretree.github.io/closure_tree/
MIT License
1.83k stars 239 forks source link

`hash_tree` giving wrong output #228

Open smoyte opened 8 years ago

smoyte commented 8 years ago

Ok this is a very tricky bug so bear with me.

I tried to create a simplified test case to illustrate it but the bug seems to depend on a very particular order of things happening that I haven't been able to reproduce inside the gem source.

Here is how to reproduce:

git clone https://github.com/sassafrastech/madeline_system.git
cd madeline_system
git checkout closure_tree_hash_tree_bug
# Ensure ruby 2.2.3 or greater
bundle install
cp config/database.yml.example config/database.yml
cp config/secrets.yml.example config/secrets.yml
rspec ./spec/models/loan_response_progress_spec.rb:29

Note the output from hash_tree that gets printed to stdout. There are three top level nodes!

Note that if you run the spec individually with rspec ./spec/models/loan_response_progress_spec.rb:53, everything looks correct. The issue seems to be created by the fixtures from earlier tests being rolled back in the database, but I'm not sure how.

I was able to trace this to an issue with the ordering of the fetched nodes. In build_hash_tree, if you echo out the fetched nodes, they are not sorted by depth, and that throws the whole thing off.

For now I was able to fix this with this commit:

https://github.com/sassafrastech/closure_tree/commit/02689da3f186b7412adf648d298afc0614123631

But I don't think that should really be necessary since self_and_descendants is supposed to return nodes in depth order but isn't.

Curious if someone can fix this properly. I think this could be a serious bug as it's not clear when the ordering of self_and_descendants is incorrect and it could affect other parts of the code.

Thanks!

smoyte commented 7 years ago

Any thoughts on this?