-
Notifications
You must be signed in to change notification settings - Fork 2
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
Command, allow to search User by Email #4
base: master
Are you sure you want to change the base?
Conversation
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.
Nice contribution thanks. Please take a look to the feedback I did.
if ($input->hasOption('role') | ||
if ($input->hasOption('email') | ||
&& null !== $input->getOption('email')) | ||
{ |
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.
wow, how did it happen like that? Please fix indentation.
if (!$find) { | ||
$output->writeln(sprintf('<error>User with role "%s" doesn\'t exist.<error>', $input->getOption('role'))); | ||
&& !in_array($input->getOption('role'), $user->getRoles())) | ||
{ |
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.
👀 same here
&& !in_array($input->getOption('role'), $user->getRoles())) | ||
{ | ||
$user = $this->findOneByRole($input, $output, $users); | ||
} |
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.
If we specify email and role, role will be ignored. This is due to refactoring in splitted methods. You probably should consider to add an error message or drop this refactoring and use a single (fat, I admit) condition.
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.
I'm assuming that if you use the 'email' flag you must know which user you want and therefore you need to know which role this user have
@megan3ss ping :) |
Add a new option for the "biig:jwt:generate"
This option allow to search an User by its Email