-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Security] Add retrieval of encompassing role names #53998
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
base: 7.3
Are you sure you want to change the base?
[Security] Add retrieval of encompassing role names #53998
Conversation
Hey! I see that this is your first PR. That is great! Welcome! Symfony has a contribution guide which I suggest you to read. In short:
Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change. When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor! I am going to sit back now and wait for the reviews. Cheers! Carsonbot |
c48453b
to
f1cc872
Compare
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.
random review :)
f1cc872
to
a21bcee
Compare
The aim of this method is to provide a handy way of getting the roles that encompass (or are parent of) an array of roles. It is similar to the getReachableRoleNames from the same interface but instead of retrieving the children roles it retrieves the parent roles. A typical use case would be when we get a user role from a database and need to get all the roles that also have access to what this role can access.
a21bcee
to
983742e
Compare
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.
+1
for renaming part, why not in a bc manner and on other PR I think :)
The aim of this method is to provide a handy way of getting the roles that encompass (or are parent of) an array of roles.
It is similar to the
RoleHierarchyInterface::getReachableRoleNames(array $roles)
but instead of retrieving the roles and children roles it retrieves the roles and parent roles.A typical use case would be when we get a user role from a database and need to get all the roles that also have access to whatever this role can access.
Also what do you guys think of renaming the existing
getReachableRoleNames
(that retrieves the "children roles" of an array of roles) as well asgetEncompassingRoleNames
(that retrieves the "parent roles" of an array of roles) togetParentRoles
andgetChildrenRoles
in order to better reflect their intention ?For the sake of this PR I tried to use a naming that is consistent with the existing
getReachableRoleNames
method.