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

[Rbac][RbacBundle] Introduce code for Rbac #3730

Merged
merged 1 commit into from
Jan 5, 2016

Conversation

tuka217
Copy link
Contributor

@tuka217 tuka217 commented Dec 16, 2015

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Fixed tickets -
License MIT
Doc PR -

@tuka217 tuka217 changed the title [WIP][Rbac][RbacBundle] Introduce code to Rbac :wq [WIP][Rbac][RbacBundle] Introduce code to Rbac Dec 16, 2015
@tuka217 tuka217 force-pushed the immutable-code-rbac branch 2 times, most recently from a1d4117 to 94d9eca Compare December 16, 2015 15:28
@tuka217 tuka217 changed the title [WIP][Rbac][RbacBundle] Introduce code to Rbac [Rbac][RbacBundle] Introduce code to Rbac Dec 16, 2015
@tuka217 tuka217 changed the title [Rbac][RbacBundle] Introduce code to Rbac [Rbac][RbacBundle] Introduce code for Rbac Dec 17, 2015
return;
}

if(!$resource instanceof RoleInterface && !$resource instanceof PermissionInterface) {
Copy link
Member

Choose a reason for hiding this comment

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

Missing space after if.

@tuka217 tuka217 force-pushed the immutable-code-rbac branch 3 times, most recently from 89216c6 to d2f1ede Compare December 17, 2015 11:34

function it_is_initializable()
{
$this->shouldHaveType(AddParentFormSubscriber::class);
Copy link
Contributor

Choose a reason for hiding this comment

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

You should use FQCN in this one instead of ::class as stated in #3736 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

👍

@pjedrzejewski
Copy link
Member

Requires rebasing.

throw new UnexpectedTypeException($resource, RoleInterface::class, PermissionInterface::class);
}

if (null === $resource->getId() XOR null === $resource->getParent()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This condition changes the previous behavior:

ConditionBefore (remove parent when true):
null !== $permission->getId() && null === $permission->getParent()
ConditionNow (add parent when false):
null === $resource->getId() XOR null === $resource->getParent()

ID Parent ResultBefore ResultNow
null null add parent add parent
null not null add parent no parent
not null null no parent no parent
not null not null add parent add parent

The proper condition should be:
null !== $resource->getId() && null === $resource->getParent()

Unless we want to change behavior?

…face

[RbacBundle] Add AddParentFormSubscriber, add this and AddCodeFormSubscriber to builder in PermissionType and RoleType
pjedrzejewski pushed a commit that referenced this pull request Jan 5, 2016
[Rbac][RbacBundle] Introduce code for Rbac
@pjedrzejewski pjedrzejewski merged commit ce57c48 into Sylius:master Jan 5, 2016
@pjedrzejewski
Copy link
Member

Thanks Ania! 👍

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.

None yet

8 participants