Skip to content
This repository was archived by the owner on Jan 30, 2020. It is now read-only.

Conversation

@traviskentbeste
Copy link

recursive check for roles

Copy link
Member

@michalbundyra michalbundyra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@traviskentbeste Thanks for your contribution. I guess you've seen my comment #45 (comment)

Your solution is not full, and if we gonna bring back recursive check we need to:

  • update also getRole method to work recursively
  • update the docs

We need also fix CS issues (please run composer cs-check and composer cs-fix for auto fixes) - Travis was not running it due to invalid env var (see my PR: #47).

$this->assertNotEquals($snafu, $this->rbac->getRole('snafu'));
$this->assertTrue($this->rbac->hasRole('snafu'));
$this->assertFalse($this->rbac->hasRole($snafu));
$this->assertTrue($this->rbac->hasRole($snafu));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is behaviour change, when we provide RoleInstance we expect that Rbac contains the same instance, so we can't compare just names, what I mean:

$role1 = new Role('role');
$role2 = new Role('role');

$role1 !== $role2;

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch! Fixed this one.

@ezimuel
Copy link
Contributor

ezimuel commented Aug 6, 2019

Closing this in favor of #48.

@ezimuel ezimuel closed this Aug 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants