-
Notifications
You must be signed in to change notification settings - Fork 465
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
Add leaves instance method #341
base: master
Are you sure you want to change the base?
Conversation
Currently working towards getting all these methods using scopes instead of moving all records through ruby. Want to hold off on this for just a little bit |
@llenodo any ideas how to do this in sql and not ruby? |
Also, I think this is going to generate way too many sql calls. |
Might be a way to do w/ SQL but would be far more complicated. IMO better to put this in now and refactor later but if you dont like the implementation feel free to reject this PR and create an open issue instead |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed sql is too much - I'll try and figure out a better test for this
@@ -35,6 +35,11 @@ def test_tree_navigation | |||
assert_equal descendants.map(&:id), lvl0_node.descendant_ids | |||
assert_equal descendants, lvl0_node.descendants | |||
assert_equal [lvl0_node] + descendants, lvl0_node.subtree | |||
# Leaves assertions | |||
leaves = lvl0_node.descendants.select do |node| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ideas how to test this without just rewriting the implementation here?
see also #246 |
Add an instance method
leaves
which returns all leaf nodes from a given node.