Completed
Push — 5.0 ( 98cec5...073169 )
by Ruud
37:30 queued 22:10
created

CreateUserCommand::interact()   D

Complexity

Conditions 13
Paths 64

Size

Total Lines 109
Code Lines 65

Duplication

Lines 32
Ratio 29.36 %

Importance

Changes 0
Metric Value
dl 32
loc 109
rs 4.9922
c 0
b 0
f 0
cc 13
eloc 65
nc 64
nop 2

How to fix   Long Method    Complexity   

Long Method

Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.

For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.

Commonly applied refactorings include:

1
<?php
2
3
namespace Kunstmaan\AdminBundle\Command;
4
5
use Doctrine\ORM\EntityManager;
6
use Kunstmaan\AdminBundle\Entity\Group;
7
use Symfony\Bundle\FrameworkBundle\Command\ContainerAwareCommand;
8
use Symfony\Component\Console\Input\ArrayInput;
9
use Symfony\Component\Console\Input\InputArgument;
10
use Symfony\Component\Console\Input\InputInterface;
11
use Symfony\Component\Console\Input\InputOption;
12
use Symfony\Component\Console\Output\OutputInterface;
13
use Symfony\Component\Console\Question\ChoiceQuestion;
14
use Symfony\Component\Console\Question\Question;
15
use Symfony\Component\Console\Exception\InvalidArgumentException;
16
17
/**
18
 * Symfony CLI command to create a user using bin/console kuma:user:create <username_of_the_user>
19
 */
20
class CreateUserCommand extends ContainerAwareCommand
21
{
22
    /** @var array */
23
    protected $groups = [];
24
25
    protected function configure()
26
    {
27
        parent::configure();
28
29
        $this->setName('kuma:user:create')
30
            ->setDescription('Create a user.')
31
            ->setDefinition(array(
32
                new InputArgument('username', InputArgument::REQUIRED, 'The username'),
33
                new InputArgument('email', InputArgument::REQUIRED, 'The email'),
34
                new InputArgument('password', InputArgument::REQUIRED, 'The password'),
35
                new InputArgument('locale', InputArgument::OPTIONAL, 'The locale (language)'),
36
                new InputOption('group', null, InputOption::VALUE_REQUIRED, 'The group(s) the user should belong to'),
37
                new InputOption('super-admin', null, InputOption::VALUE_NONE, 'Set the user as super admin'),
38
                new InputOption('inactive', null, InputOption::VALUE_NONE, 'Set the user as inactive'),
39
            ))
40
            ->setHelp(<<<EOT
41
The <info>kuma:user:create</info> command creates a user:
42
43
  <info>php bin/console kuma:user:create matthieu --group=Users</info>
44
45
This interactive shell will ask you for an email and then a password.
46
47
You can alternatively specify the email, password and locale and group as extra arguments:
48
49
  <info>php bin/console kuma:user:create matthieu [email protected] mypassword en --group=Users</info>
50
51
You can create a super admin via the super-admin flag:
52
53
  <info>php bin/console kuma:user:create admin --super-admin --group=Administrators</info>
54
55
You can create an inactive user (will not be able to log in):
56
57
  <info>php bin/console kuma:user:create thibault --inactive --group=Users</info>
58
59
<comment>Note:</comment> You have to specify at least one group.
60
61
EOT
62
            );
63
    }
64
65
    /**
66
     * Executes the current command.
67
     *
68
     * @param InputInterface $input The input
69
     * @param OutputInterface $output The output
70
     *
71
     * @return int
0 ignored issues
show
Documentation introduced by
Should the return type not be integer|null?

This check compares the return type specified in the @return annotation of a function or method doc comment with the types returned by the function and raises an issue if they mismatch.

Loading history...
72
     */
73
    protected function execute(InputInterface $input, OutputInterface $output)
74
    {
75
        /* @var EntityManager $em */
76
        $em = $this->getContainer()->get('doctrine.orm.entity_manager');
77
78
        $username = $input->getArgument('username');
79
        $email = $input->getArgument('email');
80
        $password = $input->getArgument('password');
81
        $locale = $input->getArgument('locale');
82
        $superAdmin = $input->getOption('super-admin');
83
        $inactive = $input->getOption('inactive');
84
        $groupOption = $input->getOption('group');
85
86
        if (null !== $locale) {
87
            $locale = $this->getContainer()->getParameter('kunstmaan_admin.default_admin_locale');
88
        }
89
        $command = $this->getApplication()->find('fos:user:create');
90
        $arguments = array(
91
            'command' => 'fos:user:create',
92
            'username' => $username,
93
            'email' => $email,
94
            'password' => $password,
95
            '--super-admin' => $superAdmin,
96
            '--inactive' => $inactive,
97
        );
98
99
        $input = new ArrayInput($arguments);
100
        $command->run($input, $output);
101
102
        // Fetch user that was just created
103
        $userClassName = $this->getContainer()->getParameter('fos_user.model.user.class');
104
        $user = $em->getRepository($userClassName)->findOneBy(array('username' => $username));
105
106
        // Attach groups
107
        $groupOutput = [];
108
109
        foreach (explode(',', $groupOption) as $groupId) {
110
            $group = $this->groups[$groupId];
111
            $groupOutput[] = $group->getName();
112
113
            if ($group instanceof Group) {
114
                $user->getGroups()->add($group);
115
            }
116
        }
117
118
        // Set admin interface locale and enable password changed
119
        $user->setAdminLocale($locale);
120
        $user->setPasswordChanged(true);
121
122
        // Persist
123
        $em->persist($user);
0 ignored issues
show
Bug introduced by
It seems like $user defined by $em->getRepository($user...sername' => $username)) on line 104 can also be of type null; however, Doctrine\ORM\EntityManager::persist() does only seem to accept object, maybe add an additional type check?

If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check:

/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}

If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue.

Loading history...
124
        $em->flush();
125
126
        $output->writeln(sprintf('Added user <comment>%s</comment> to groups <comment>%s</comment>', $input->getArgument('username'), implode(',', $groupOutput)));
127
    }
128
129
    /**
130
     * Interacts with the user.
131
     *
132
     * @param InputInterface $input The input
133
     * @param OutputInterface $output The output
134
     *
135
     * @throws \InvalidArgumentException
136
     *
137
     * @return void
138
     */
139
    protected function interact(InputInterface $input, OutputInterface $output)
140
    {
141 View Code Duplication
        if (!$input->getArgument('username')) {
0 ignored issues
show
Duplication introduced by
This code seems to be duplicated across your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

You can also find more detailed suggestions in the “Code” section of your repository.

Loading history...
142
            $question = New Question('Please choose a username:');
143
            $question->setValidator(function ($username) {
144
                if (null === $username) {
145
                    throw new \InvalidArgumentException('Username can not be empty');
146
                }
147
148
                return $username;
149
            });
150
            $username = $this->getHelper('question')->ask(
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Symfony\Component\Console\Helper\HelperInterface as the method ask() does only exist in the following implementations of said interface: Sensio\Bundle\GeneratorB...d\Helper\QuestionHelper, Symfony\Component\Console\Helper\QuestionHelper, Symfony\Component\Consol...r\SymfonyQuestionHelper.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
151
                $input,
152
                $output,
153
                $question
154
            );
155
            $input->setArgument('username', $username);
156
        }
157
158 View Code Duplication
        if (!$input->getArgument('email')) {
0 ignored issues
show
Duplication introduced by
This code seems to be duplicated across your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

You can also find more detailed suggestions in the “Code” section of your repository.

Loading history...
159
            $question = New Question('Please choose an email:');
160
            $question->setValidator(function ($email) {
161
                if (null === $email) {
162
                    throw new \InvalidArgumentException('Email can not be empty');
163
                }
164
165
                return $email;
166
            });
167
            $email = $this->getHelper('question')->ask(
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Symfony\Component\Console\Helper\HelperInterface as the method ask() does only exist in the following implementations of said interface: Sensio\Bundle\GeneratorB...d\Helper\QuestionHelper, Symfony\Component\Console\Helper\QuestionHelper, Symfony\Component\Consol...r\SymfonyQuestionHelper.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
168
                $input,
169
                $output,
170
                $question
171
            );
172
            $input->setArgument('email', $email);
173
        }
174
175
        if (!$input->getArgument('password')) {
176
177
            $question = New Question('Please choose a password:');
178
            $question->setHidden(true);
179
            $question->setHiddenFallback(false);
180
            $question->setValidator(function ($password) {
181
                if (null === $password) {
182
                    throw new \InvalidArgumentException('Password can not be empty');
183
                }
184
185
                return $password;
186
            });
187
            $password = $this->getHelper('question')->ask(
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Symfony\Component\Console\Helper\HelperInterface as the method ask() does only exist in the following implementations of said interface: Sensio\Bundle\GeneratorB...d\Helper\QuestionHelper, Symfony\Component\Console\Helper\QuestionHelper, Symfony\Component\Consol...r\SymfonyQuestionHelper.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
188
                $input,
189
                $output,
190
                $question
191
            );
192
193
            $input->setArgument('password', $password);
194
        }
195
196
        if (!$input->getArgument('locale')) {
197
            $locale = $this->getHelper('question')->ask(
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Symfony\Component\Console\Helper\HelperInterface as the method ask() does only exist in the following implementations of said interface: Sensio\Bundle\GeneratorB...d\Helper\QuestionHelper, Symfony\Component\Console\Helper\QuestionHelper, Symfony\Component\Consol...r\SymfonyQuestionHelper.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
198
                $input,
199
                $output,
200
                new Question('Please enter the locale (or leave empty for default admin locale):')
201
            );
202
            $input->setArgument('locale', $locale);
203
        }
204
205
        $this->groups = $this->getContainer()->get('fos_user.group_manager')->findGroups();
206
207
        // reindexing the array, using the db id as the key
208
        $newGroups = [];
209
        foreach($this->groups as $group) {
210
            $newGroups[$group->getId()] = $group;
211
        }
212
213
        $this->groups = $newGroups;
214
215
        if (!$input->getOption('group')) {
216
            $question = new ChoiceQuestion(
217
                'Please enter the group(s) the user should be a member of (multiple possible, separated by comma):',
218
                $this->groups,
219
                ''
220
            );
221
            $question->setMultiselect(true);
222
            $question->setValidator(function ($groupsInput) {
223
224
                if (!$this->groups) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $this->groups of type array is implicitly converted to a boolean; are you sure this is intended? If so, consider using empty($expr) instead to make it clear that you intend to check for an array without elements.

This check marks implicit conversions of arrays to boolean values in a comparison. While in PHP an empty array is considered to be equal (but not identical) to false, this is not always apparent.

Consider making the comparison explicit by using empty(..) or ! empty(...) instead.

Loading history...
225
                    throw new \RuntimeException('No user group(s) could be found');
226
                }
227
228
                // Validate that the chosen group options exist in the available groups
229
                $groupNames = array_unique(explode(',', $groupsInput));
230
                if (count(array_intersect_key(array_flip($groupNames),$this->groups)) !== count($groupNames)) {
231
                    throw new InvalidArgumentException('You have chosen non existing group(s)');
232
                }
233
234
                if ($groupsInput === '') {
235
                    throw new \RuntimeException(
236
                        'Group(s) must be of type integer and can not be empty'
237
                    );
238
                }
239
                return $groupsInput;
240
            });
241
242
            // Group has to be imploded because $input->setOption expects a string
243
            $groups = $this->getHelper('question')->ask($input, $output, $question);
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Symfony\Component\Console\Helper\HelperInterface as the method ask() does only exist in the following implementations of said interface: Sensio\Bundle\GeneratorB...d\Helper\QuestionHelper, Symfony\Component\Console\Helper\QuestionHelper, Symfony\Component\Consol...r\SymfonyQuestionHelper.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
244
245
            $input->setOption('group', $groups);
246
        }
247
    }
248
}
249