New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New multi-trees test, failing #146

Merged
merged 2 commits into from Sep 7, 2015

Conversation

Projects
None yet
2 participants
@mkralla11
Copy link
Contributor

mkralla11 commented Feb 2, 2015

I ran into the issue that has occurred for multiple programmers.
Issues:
#37 (Different issue: this is .hash_tree from a non-tree relation)
#76 (Different issue: this was fixed many versions ago, and has test coverage)
#28 (Different issue: this was fixed many versions ago, and has test coverage)

The issue is occurring because the call to hash_tree is on an already-scoped ActiveRecord relation, and your previous tests only cover Class.hash_tree.

example:

d = Tag.find_or_create_by_path %w[a b c d]
h = Tag.find_or_create_by_path %w[e f g h]

Tag.first.children.hash_tree

NoMethodError - undefined method `[]=' for nil:NilClass:
  (gem) closure_tree-5.1.1/lib/closure_tree/hash_tree.rb:52:in `block in build_hash_tree'
  (gem) activerecord-4.1.0/lib/active_record/relation/delegation.rb:46:in `each'
  (gem) closure_tree-5.1.1/lib/closure_tree/hash_tree.rb:47:in `build_hash_tree'
  (gem) closure_tree-5.1.1/lib/closure_tree/hash_tree.rb:23:in `hash_tree'
  (gem) activerecord-4.1.0/lib/active_record/relation/delegation.rb:94:in `block in method_missing'
  (gem) activerecord-4.1.0/lib/active_record/relation.rb:286:in `scoping'
  (gem) activerecord-4.1.0/lib/active_record/relation/delegation.rb:94:in `method_missing'
  app/controllers/aging_resources_controller.rb:34:in `load_resource'

I have updated all your tests to accommodate for the desired/intended behavior, and they all pass. I then wrote my own test that demonstrates the issue occurring and the test fails.

Let me know if you need any more info from me. The issue is occurring on line 52 of /lib/closure_tree/hash_tree.rb and it seems like there needs to be another conditional statement on line 49 to fix the problem, but I haven't had time to throw bricks at it yet. It might be an easy fix for you, @mceachen, so I thought I'd let you take a look.

Btw, I love this gem :). Nice work!

@mceachen

This comment has been minimized.

Copy link
Collaborator

mceachen commented Feb 3, 2015

Thanks for making those failing tests! I'll try to check them out when I've got a moment this next weekend.

@mceachen

This comment has been minimized.

Copy link
Collaborator

mceachen commented Feb 8, 2015

Ah, so I found the actual issue here—the node.children.hash_tree passes a scope that has a where condition (where the parent_id is the id of node)—which causes only the children to be returned from hash_tree.

@mceachen mceachen merged commit 9c71d4f into ClosureTree:master Sep 7, 2015

1 check failed

continuous-integration/travis-ci The Travis CI build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment