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 recursive capability to isDescendantOf & isAncestorOf #127

Closed
kkatpcc opened this issue May 1, 2020 · 2 comments
Closed

Add recursive capability to isDescendantOf & isAncestorOf #127

kkatpcc opened this issue May 1, 2020 · 2 comments
Labels
bug Something isn't working

Comments

@kkatpcc
Copy link

kkatpcc commented May 1, 2020

Is your feature request related to a problem? Please describe.
The current implementation of the isDescendantOf and isAncestorOf methods of the LdapRecord\Models\Model class are non-recursive. While the documentation indeed indicates this as the intended behavior, such usage is not intuitive with respect to the usual unbounded meaning of the terms descendant and ancestor.

Describe the solution you'd like
In order not to break existing code, I would like to initially suggest adding an optional second boolean parameter to the aforementioned methods representing whether the comparison should be recursive or not, which would default to the current non-recursive behavior, e.g. $recursive = false.

Describe alternatives considered
One alternative would be to change the behavior of isDescendantOf and isAncestorOf to be always recursive, while adding something like isChildOf and isParentOf for the non-recursive counterparts. This could be combined with the above initial suggestion and be phased in over time, perhaps with deprecation notices, as not to catch users by surprise.

Additional context
Related commit comment: f2a4a3a#commitcomment-38808641

@stevebauman stevebauman added the bug Something isn't working label May 1, 2020
@stevebauman
Copy link
Member

stevebauman commented May 1, 2020

Hi @kkatpcc, thanks! Appreciate the creation of the issue.

I'm going to consider this a bug so I'm going to modify the behaviour of isDescendantOf and isAncestorOf so they work properly and return the expected result.

I'm also going to add your suggested methods isChildOf and isParentOf that preserves the current behaviour.

Give me a couple hours and I'll have this fixed 👍

@stevebauman
Copy link
Member

Completed.

  • Model::isDescendantOf and Model::isAncestorOf are now recursive
  • Model::isChildOf and Model::isParentOf have been added to replace the current behaviour
  • DistinguishedName attribute class has been added to perform these comparative operations

New release will be out shortly with these fixes 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants