Skip to content

Lowest common ancestor #326

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

Closed

Conversation

robotdana
Copy link
Contributor

For finding the lowest common ancestor of a relation

@seuros
Copy link
Member

seuros commented Sep 24, 2018

@robotdana Thank you for your contribution. Can you give an use case for this feature ?
Also i see that you are unscoping the default scope. It will be problimatic for people that have default scopes that deleted records

@robotdana
Copy link
Contributor Author

robotdana commented Sep 25, 2018

Our use case is finding the root to build navigation for a subset of records

lowest_common_ancestor is the most common treeish name for it

The default scope is easy to reapply, i've added a commit fixing it

But,default_scoped is an undocumented rails method, and this gem already has a warning about using default_scope (the only place the whole gem mentions default_scope)

### Does this work well with ```#default_scope```?

**No.** Please see [issue 86](https://github.com/ClosureTree/closure_tree/issues/86) for details.

So i initially thought the simpler version better

A version without using unscoped is over here with a different signature: master...robotdana:lowest_common_ancestor_non_chain

@robotdana robotdana force-pushed the lowest_common_ancestor branch from 3396490 to 3ec195a Compare September 25, 2018 00:07
@robotdana
Copy link
Contributor Author

default scoped is an undocumented rails method, and it looks like it works differently in different versions, closing because #328 is a safer version of this

@robotdana robotdana closed this Sep 25, 2018
@robotdana robotdana deleted the lowest_common_ancestor branch September 26, 2018 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants