[UserBundle] adding commands to promote or demote a user by adding or removing a role #4212
Conversation
loic425
commented
Feb 18, 2016
loic425
commented
| Q | A |
|---|---|
| Bug fix? | no |
| New feature? | yes |
| BC breaks? | no |
| Deprecations? | no |
| Fixed tickets | |
| License | MIT |
| Doc PR |
pjedrzejewski
added
DX
| if (empty($email)) { | ||
| throw new \Exception('Email can not be empty'); | ||
| } | ||
| return $email; |
There was a problem hiding this comment.
These lines are a copy/paste from existing "CreateUserCommand".
There was a problem hiding this comment.
Still, there should be a blank line before return statement. See
There was a problem hiding this comment.
Ok, my bad. I'm gonna fix that.
|
@pjedrzejewski Theses 2 commands handle with RBAC roles now. I omitted this concept. |
|
|
||
| /** | ||
| * @param string $role | ||
| * @return RoleInterface |
|
@pjedrzejewski I've done all changes from lchrusciel review. |
|
@pjedrzejewski @lchrusciel |
| /** | ||
| * @author Loïc Frémont <loic@mobizel.com> | ||
| */ | ||
| interface RoleCommandInterface |
There was a problem hiding this comment.
What's the purpose of this interface? You do not inject it anywhere.
There was a problem hiding this comment.
AbstractRoleCommand object use this interface. This command calls "executeRoleCommand" method. Thus, to extend AbstractRoleCommand, you need to add executeRoleCommand method.
| */ | ||
| protected function findAuthorizationRole($role) | ||
| { | ||
| $roleRepository = $this->getContainer()->get('sylius.repository.role'); |
There was a problem hiding this comment.
This couples SyliusUserBundle to SyliusRbacBundle, which is something we would like to avoid. Sorry, I just noticed it. I think there should be a separate command in RbacBundle to do this.
There was a problem hiding this comment.
@pjedrzejewski
Ok, I see. The actual CreateUserCommand has the same problem.
I'm gonna work (if you agree) on two commands, one for SyliusUserBundle and one for SyliusRbacBundle.
I can also work on another pull request to improve CreateUserCommand.
|
This coupling to RbacBundle is a blocker atm. :/ We cannot use |
|
@pjedrzejewski |
| ->setHelp(<<<EOT | ||
| The <info>sylius:user:demote</info> command demotes a user by removing security roles | ||
|
|
||
| <info>php app/console fos:user:demote matthieu@email.com</info> |
|
Thank you @loic425! |
