DeleteCommand::batchDelete()   A
last analyzed

Complexity

Conditions 3
Paths 3

Size

Total Lines 11

Duplication

Lines 0
Ratio 0 %

Importance

Changes 0
Metric Value
cc 3
nc 3
nop 1
dl 0
loc 11
rs 9.9
c 0
b 0
f 0
1
<?php
2
3
namespace N98\Magento\Command\Customer;
4
5
use Exception;
6
use Mage_Customer_Model_Entity_Customer_Collection;
7
use Mage_Customer_Model_Resource_Customer_Collection;
8
use N98\Util\Console\Helper\ParameterHelper;
9
use RuntimeException;
10
use Symfony\Component\Console\Helper\DialogHelper;
11
use Symfony\Component\Console\Input\InputArgument;
12
use Symfony\Component\Console\Input\InputInterface;
13
use Symfony\Component\Console\Input\InputOption;
14
use Symfony\Component\Console\Output\OutputInterface;
15
16
/**
17
 * Class DeleteCommand
18
 * @package N98\Magento\Command\Customer
19
 */
20
class DeleteCommand extends AbstractCustomerCommand
21
{
22
    /**
23
     * @var InputInterface
24
     */
25
    protected $input;
26
27
    /**
28
     * @var OutputInterface
29
     */
30
    protected $output;
31
32
    /**
33
     * @var DialogHelper
34
     */
35
    protected $dialog;
36
37
    /**
38
     * Set up options
39
     */
40
    protected function configure()
41
    {
42
        $this
43
            ->setName('customer:delete')
44
            ->addArgument('id', InputArgument::OPTIONAL, 'Customer Id or email', false)
45
            ->addOption('all', 'a', InputOption::VALUE_NONE, 'Delete all customers')
46
            ->addOption('force', 'f', InputOption::VALUE_NONE, 'Force delete')
47
            ->addOption('range', '-r', InputOption::VALUE_NONE, 'Delete a range of customers by Id')
48
            ->setDescription('Delete Customer/s');
49
50
        $help = <<<HELP
51
This will delete a customer by a given Id/Email, delete all customers or delete all customers in a range of Ids.
52
53
<comment>Example Usage:</comment>
54
55
n98-magerun customer:delete 1                   <info># Will delete customer with Id 1</info>
56
n98-magerun customer:delete [email protected]    <info># Will delete customer with that email</info>
57
n98-magerun customer:delete --all               <info># Will delete all customers</info>
58
n98-magerun customer:delete --range             <info># Will prompt for start and end Ids for batch deletion</info>
59
60
HELP;
61
62
        $this->setHelp($help);
63
    }
64
65
    /**
66
     * @param InputInterface  $input
67
     * @param OutputInterface $output
68
     *
69
     * @return false|null
70
     */
71
    protected function execute(InputInterface $input, OutputInterface $output)
72
    {
73
        $this->detectMagento($output, true);
74
        if (!$this->initMagento()) {
75
            return;
76
        }
77
78
        $this->input = $input;
79
        $this->output = $output;
80
        /** @var DialogHelper dialog */
81
        $this->dialog = $this->getHelper('dialog');
82
83
        // Defaults
84
        $range = $all = false;
0 ignored issues
show
Unused Code introduced by
$all is not used, you could remove the assignment.

This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.

$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}

Both the $myVar assignment in line 1 and the $higher assignment in line 2 are dead. The first because $myVar is never used and the second because $higher is always overwritten for every possible time line.

Loading history...
Unused Code introduced by
$range is not used, you could remove the assignment.

This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.

$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}

Both the $myVar assignment in line 1 and the $higher assignment in line 2 are dead. The first because $myVar is never used and the second because $higher is always overwritten for every possible time line.

Loading history...
85
86
        $id = $this->input->getArgument('id');
87
        $range = $this->input->getOption('range');
88
        $all = $this->input->getOption('all');
89
        // Get args required
90
        if (!($id) && !($range) && !($all)) {
91
92
            // Delete more than one customer ?
93
            $batchDelete = $this->dialog->askConfirmation(
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 askConfirmation() does only exist in the following implementations of said interface: Symfony\Component\Console\Helper\DialogHelper.

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...
94
                $this->output,
95
                $this->getQuestion('Delete more than 1 customer?', 'n'),
96
                false
97
            );
98
99
            if ($batchDelete) {
100
                // Batch deletion
101
                $all = $this->dialog->askConfirmation(
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 askConfirmation() does only exist in the following implementations of said interface: Symfony\Component\Console\Helper\DialogHelper.

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...
102
                    $this->output,
103
                    $this->getQuestion('Delete all customers?', 'n'),
104
                    false
105
                );
106
107
                if (!$all) {
108
                    $range = $this->dialog->askConfirmation(
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 askConfirmation() does only exist in the following implementations of said interface: Symfony\Component\Console\Helper\DialogHelper.

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...
109
                        $this->output,
110
                        $this->getQuestion('Delete a range of customers?', 'n'),
111
                        false
112
                    );
113
114
                    if (!$range) {
115
                        // Nothing to do
116
                        $this->output->writeln('<error>Finished nothing to do</error>');
117
                        return false;
0 ignored issues
show
Bug Best Practice introduced by
The return type of return false; (false) is incompatible with the return type of the parent method Symfony\Component\Console\Command\Command::execute of type null|integer.

If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.

Let’s take a look at an example:

class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}

Our function my_function expects a Post object, and outputs the author of the post. The base class Post returns a simple string and outputting a simple string will work just fine. However, the child class BlogPost which is a sub-type of Post instead decided to return an object, and is therefore violating the SOLID principles. If a BlogPost were passed to my_function, PHP would not complain, but ultimately fail when executing the strtoupper call in its body.

Loading history...
118
                    }
119
                }
120
            }
121
        }
122
123
        if (!$range && !$all) {
124
            // Single customer deletion
125
            if (!$id) {
126
                $id = $this->dialog->ask($this->output, $this->getQuestion('Customer Id'), null);
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: Symfony\Component\Console\Helper\DialogHelper, 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...
127
            }
128
129
            try {
130
                $customer = $this->getCustomer($id);
131
            } catch (Exception $e) {
132
                $this->output->writeln('<error>No customer found!</error>');
133
                return false;
0 ignored issues
show
Bug Best Practice introduced by
The return type of return false; (false) is incompatible with the return type of the parent method Symfony\Component\Console\Command\Command::execute of type null|integer.

If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.

Let’s take a look at an example:

class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}

Our function my_function expects a Post object, and outputs the author of the post. The base class Post returns a simple string and outputting a simple string will work just fine. However, the child class BlogPost which is a sub-type of Post instead decided to return an object, and is therefore violating the SOLID principles. If a BlogPost were passed to my_function, PHP would not complain, but ultimately fail when executing the strtoupper call in its body.

Loading history...
134
            }
135
136
            if ($this->shouldRemove()) {
137
                $this->deleteCustomer($customer);
138
            } else {
139
                $this->output->writeln('<error>Aborting delete</error>');
140
            }
141
        } else {
142
            $customers = $this->getCustomerCollection();
143
            $customers
144
                ->addAttributeToSelect('firstname')
145
                ->addAttributeToSelect('lastname')
146
                ->addAttributeToSelect('email');
147
148
            if ($range) {
149
                // Get Range
150
                $ranges = array();
151
                $ranges[0] = $this->dialog->askAndValidate(
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 askAndValidate() does only exist in the following implementations of said interface: N98\Util\Console\Helper\ParameterHelper, Symfony\Component\Console\Helper\DialogHelper.

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...
152
                    $this->output,
153
                    $this->getQuestion('Range start Id', '1'),
154
                    array($this, 'validateInt'),
155
                    false,
156
                    '1'
157
                );
158
                $ranges[1] = $this->dialog->askAndValidate(
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 askAndValidate() does only exist in the following implementations of said interface: N98\Util\Console\Helper\ParameterHelper, Symfony\Component\Console\Helper\DialogHelper.

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...
159
                    $this->output,
160
                    $this->getQuestion('Range end Id', '1'),
161
                    array($this, 'validateInt'),
162
                    false,
163
                    '1'
164
                );
165
166
                // Ensure ascending order
167
                sort($ranges);
168
169
                // Range delete, takes precedence over --all
170
                $customers->addAttributeToFilter('entity_id', array(
171
                    'from'  => $ranges[0],
172
                    'to'    => $ranges[1],
173
                ));
174
            }
175
176
            if ($this->shouldRemove()) {
177
                $count = $this->batchDelete($customers);
178
                $this->output->writeln('<info>Successfully deleted ' . $count . ' customer/s</info>');
179
            } else {
180
                $this->output->writeln('<error>Aborting delete</error>');
181
            }
182
        }
183
    }
184
185
    /**
186
     * @return bool
187
     */
188
    protected function shouldRemove()
189
    {
190
        $shouldRemove = $this->input->getOption('force');
191
        if (!$shouldRemove) {
192
            $shouldRemove = $this->dialog->askConfirmation(
193
                $this->output,
194
                $this->getQuestion('Are you sure?', 'n'),
195
                false
196
            );
197
        }
198
199
        return $shouldRemove;
200
    }
201
202
    /**
203
     * @param int|string $id
204
     *
205
     * @return \Mage_Customer_Model_Customer
206
     * @throws RuntimeException
207
     */
208
    protected function getCustomer($id)
209
    {
210
        /** @var \Mage_Customer_Model_Customer $customer */
211
        $customer = $this->getCustomerModel()->load($id);
212
        if (!$customer->getId()) {
213
            /** @var $parameterHelper ParameterHelper */
214
            $parameterHelper = $this->getHelper('parameter');
215
            $website = $parameterHelper->askWebsite($this->input, $this->output);
216
            $customer = $this->getCustomerModel()
217
                ->setWebsiteId($website->getId())
218
                ->loadByEmail($id);
219
        }
220
221
        if (!$customer->getId()) {
222
            throw new RuntimeException('No customer found!');
223
        }
224
225
        return $customer;
226
    }
227
228
    /**
229
     * @param \Mage_Customer_Model_Customer $customer
230
     *
231
     * @return true|Exception
232
     */
233
    protected function deleteCustomer(\Mage_Customer_Model_Customer $customer)
234
    {
235
        try {
236
            $customer->delete();
237
            $this->output->writeln(
238
                sprintf('<info>%s (%s) was successfully deleted</info>', $customer->getName(), $customer->getEmail())
239
            );
240
            return true;
241
        } catch (Exception $e) {
242
            $this->output->writeln('<error>' . $e->getMessage() . '</error>');
243
            return $e;
244
        }
245
    }
246
247
    /**
248
     * @param Mage_Customer_Model_Entity_Customer_Collection|Mage_Customer_Model_Resource_Customer_Collection $customers
249
     *
250
     * @return int
251
     */
252
    protected function batchDelete($customers)
253
    {
254
        $count = 0;
255
        foreach ($customers as $customer) {
256
            if ($this->deleteCustomer($customer) === true) {
257
                $count++;
258
            }
259
        }
260
261
        return $count;
262
    }
263
264
    /**
265
     * @param string $answer
266
     * @return string
267
     */
268
    public function validateInt($answer)
269
    {
270
        if (intval($answer) === 0) {
271
            throw new RuntimeException(
272
                'The range should be numeric and above 0 e.g. 1'
273
            );
274
        }
275
276
        return $answer;
277
    }
278
279
    /**
280
     * @param string $message
281
     * @param string $default [optional]
282
     *
283
     * @return string
284
     */
285
    private function getQuestion($message, $default = null)
286
    {
287
        $params = array($message);
288
        $pattern = '%s: ';
289
290
        if (null !== $default) {
291
            $params[] = $default;
292
            $pattern .= '[%s] ';
293
        }
294
295
        return vsprintf($pattern, $params);
296
    }
297
}
298