Skip to content
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 siblings-only finders (excluding the targeted node) #285

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

chikamichi
Copy link

@chikamichi chikamichi commented Sep 17, 2016

Hi,

This PR adds support for siblings-only finders and scope, eg. #siblings_only mainly.

Excluding the targeted node from the list of its siblings is as simple as node.siblings.reject { |s| s == node }, but it might make more sense to handle that within ancestry itself so as to leverage a cleaner, more efficient SQL query, and to provide a full-fledged API (eg. Model.siblings_only_of(node), Model#siblings_only_ids).

@coveralls
Copy link

coveralls commented Sep 17, 2016

Coverage Status

Coverage increased (+0.05%) to 98.165% when pulling b851130 on chikamichi:feature/siblings-only into 68d6c05 on stefankroes:master.

@chikamichi
Copy link
Author

Specs actually are passing, something's wrong on Travis for Ruby 1.9.3 when it comes to the config and some gems cannot be installed:

An error occurred while installing {foobar} (x.y.z), and Bundler cannot continue.

@kbrock
Copy link
Collaborator

kbrock commented May 26, 2021

@d-m-u wondering

  • does it make sense to make siblings a configuration setting?
  • is there an agreed upon standard for what you call all my parent's children?
  • I think I like the simpler query and count > 1 vs complex query > 0

@kbrock
Copy link
Collaborator

kbrock commented Jan 27, 2023

related to #488

@kbrock
Copy link
Collaborator

kbrock commented Jan 27, 2023

I'm thinking this should be a breaking change and a config parameter would switch between the two?

thinking out loud and would love any feedback.

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.

3 participants