My old Magento code review – Why you never stop learning

What made me look at my old, dusty code

Recently I got a comment below one of my older articles saying that a solution I proposed is no longer working in a newer Magento version. I didn’t really remember much about what I wrote in there, so I skimmed through the article and froze. Did I really write this? The article is three years old and at that point, I had considered myself an experienced developer, capable of writing guides and articles. The reality, it appears, was very different. I concluded I’m not able to help the author of the comment and decided it’s about time to completely rewrite the article, focusing on my lack of knowledge and a proper approach that has evolved during the three years of new experiences. I invite you to join me on my joyride where I’ll be doing my own Magento code review :)

The article in question is How to Create Customers Using CLI in Magento 2. I invite you to join me on my joyride where I’ll be doing my own Magento code review :) I’ll try to present it in a way to show the evolution of my approach to development but also provide better actual functionality of creating customer accounts with CLI.

Creating the module

Let’s look at the old intro:

Compared to Magento 1, Magento 2 has a built-in store control from the console. In order to see all the commands available in our store, we need to run the following command from the main folder: $ php bin/magento

They help to automate tasks and allow us quick access to the basic functionalities, i.e. clearing cache, reindexing, or adding an admin account. The commands are nothing else than an executed PHP script. Magento enables us to easily create our own command and this is what we want to do here.

In today’s article we’ll see how to use the console to create a customer account with our custom command.

It is actually quite alright. I would only add that there is a custom CLI solution in Magento 1, namely n98-magerun (also available for Magento 2):

GitHub - netz98/n98-magerun: The swiss army knife for Magento developers, sysadmins and devops. The tool provides a huge set of well-tested command line commands which save hours of work time. All commands are extendable by a module API.

Regarding creating the module, this code chunk is still valid:

php
1 2 3 4 5 6 7 8 //file: app/code/Magently/Customer/registration.php <?php \Magento\Framework\Component\ComponentRegistrar::register( \Magento\Framework\Component\ComponentRegistrar::MODULE, 'Magently_Customer', __DIR__ );

However, the etc/module.xml file definition has changed.

Before:
(to see complete code snippets, visit the original article).

Now:

html
1 2 3 4 5 6 7 8 9 10 11 //file: app/code/Magently/Customer/etc/module.xml <?xml version="1.0"?> <config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:Module/etc/module.xsd"> <module name="Magently_Customer" setup_version="0.1.0"> <sequence> <module name="Magento_Customer"/> </sequence> </module> </config>

Even on such a simple step that is the module initialization, there are some differences:

  • first, properly formatted long lines. I’ve been wary of this issue for some time now and pay attention during CR to break long XML markers to separate lines, like xsi:noNamespaceSchemaLocation in this example. It improves legibility because the code doesn’t exceed 120 signs;
  • another thing is pointing at the localization of the XML definition, namely module.xsd file. I have no idea why would I use relative paths there. Most likely I just found and copied a solution from a different article. Looking at Magento GitHub, urn is supported as of version 2.0.0 and relative paths have been obsolete since the beta;
  • yet another thing I forgot about in the original article is the loading sequence of the modules. When debugging, I used to repeatedly wonder why in the world my XML files don’t merge properly. Having a sequence also helps with readability because we know the purpose of the module.

What’s more, the module initialization lacks the composer.json file but if we build the module in app/code, it’s not really required.

Creating a Magento command

Moving forward, we’re initializing a new command:

Adding a new command to CLI is based on passing on the argument from the XML level to the class Magento\Framework\Console\CommandList. Dependency Injection comes in handy here. Let’s create the file app/code/Magently/Customer/etc/di.xml with the following content.

You can learn more about DI here and here.

Initializing a new command in the etc/di.xml file has changed as well. Before:

Now:

html
1 2 3 4 5 6 7 8 9 10 11 12 <?xml version="1.0"?> <config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:ObjectManager/etc/config.xsd"> <type name="Magento\Framework\Console\CommandListInterface"> <arguments> <argument name="commands" xsi:type="array"> <item name="magently_customer_user_create" xsi:type="object">Magently\Customer\Console\Command\CustomerAccountCreate</item> </argument> </arguments> </type> </config>

Here, for some reason, I did use urn. What’s changed is a place when we’re adding the command - an interface instead of a class. The original solution will still work for you, as long as someone doesn’t get too ambitious and create a new implementation of the CommandListInterface interface. Apart from formatting, I changed the index name of the element passed to the commands table, i.e. I added magently prefix to avoid collision with another element of the same name. I also changed the name of the class that creates the command item from CustomerUserCreateCommand to CustomerAccountCreate - I decided that the mere fact of the class existing in a namespace that has Console/Command makes it clear enough what type of class are we dealing with.

Creating a customer

Now here is where it gets really interesting. Below you’ll see the code of two classes that were responsible for creating the customer’s account. The only thing I could be proud of is that I didn’t make it a single huge class and that I almost managed to separate logic from the input/output data.
(to see complete code snippets, visit the original article).

Let me mercifully skip mentioning the lack of any comments or cohesion in using use. What mistakes did I make in the old implementation? First of all, the command class and its execution weren’t very coherent. I passed the data to the class that creates customers’ accounts only to list it basing on the input, ignoring the fact that the data could have very well been changed somewhere along the line, e.g. by a plugin modifying email/firstname/whatever for us:

Meanwhile, in a class responsible for creating customers’ accounts, I made it impossible for it to be used in a different context (e.g. in a simple form on the backend). I hardcoded area code to frontend and if I ever tried to use this class somewhere else, we would be greeted by the Area code is already set exception. At least I didn’t make it in the constructor :)

Another thing is linking the class behavior to the specific data passing logic. Because I made the Input object required, I made it impossible to use any other data. What’s more, I saved the data in the object properties, what could’ve caused data desynchronization if anyone tried to use the class somewhere else after it’s been executed (of course I could have just recommended using Factory).

Another bad practice was using the save() method on the model. The method works as intended, that is it loads its Resource Model and performs a save on the database, but it’s been deprecated for a long time now. And it’s been deprecated because the data model should only be responsible for data and not for the logic that dictates how it should be saved. What’s more, Magento offers special classes for creating user accounts that give way better possibilities and I think it is best to use them. I’m not saying you should always follow Magento core when choosing solutions. It’s much better to treat them as suggestions because the core code itself is not flawless.

As a cherry on top, you can tell the code above did not pass any code review because there are a leftover property and the Store Manager object that aren’t used anywhere in the class.

So here is how the code would look if I created it today:

php
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 // file app/code/Magently/Customer/Console/Command/CustomerAccountCreate.php <?php namespace Magently\Customer\Console\Command; use Magento\Framework\Exception\LocalizedException; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Input\InputOptionFactory; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; use Magento\Framework\DataObjectFactory; use Magently\Customer\Model\NewCustomer; use Magento\Framework\App\State; use Magento\Framework\App\Area; /** * Class CustomerAccountCreate * The class responsible for providing a command to create a customer account */ class CustomerAccountCreate extends Command { /** * @var InputOptionFactory */ private $inputOptionFactory; /** * @var NewCustomer */ private $newCustomer; /** * @var DataObjectFactory */ private $dataObjectFactory; /** * @var State */ private $appState; /** * CustomerAccountCreate constructor. * @param InputOptionFactory $inputOptionFactory * @param DataObjectFactory $dataObjectFactory * @param State $appState * @param NewCustomer $newCustomer * @param null|string $name */ public function __construct( InputOptionFactory $inputOptionFactory, DataObjectFactory $dataObjectFactory, State $appState, NewCustomer $newCustomer, $name = null ) { $this->inputOptionFactory = $inputOptionFactory; $this->dataObjectFactory = $dataObjectFactory; $this->newCustomer = $newCustomer; $this->appState = $appState; parent::__construct($name); } /** * {@inheritDoc} * @return void */ protected function configure() { $this->setName('customer:account:create') ->setDescription('Create new customer') ->setDefinition($this->getOptionsList()); } /** * {@inheritDoc} * @param InputInterface $input * @param OutputInterface $output * @return voi * @throws LocalizedException */ protected function execute(InputInterface $input, OutputInterface $output) { $output->writeln('<info>Creating new user...</info>'); $customerData = $this->dataObjectFactory->create([ 'data' => [ NewCustomer::KEY_FIRSTNAME => $input->getOption(NewCustomer::KEY_FIRSTNAME), NewCustomer::KEY_LASTNAME => $input->getOption(NewCustomer::KEY_LASTNAME), NewCustomer::KEY_EMAIL => $input->getOption(NewCustomer::KEY_EMAIL), NewCustomer::KEY_PASSWORD => $input->getOption(NewCustomer::KEY_PASSWORD), NewCustomer::KEY_WEBSITE => $input->getOption(NewCustomer::KEY_WEBSITE), ] ]); $customer = $this->appState->emulateAreaCode(Area::AREA_FRONTEND, function () use ($customerData) { return $this->newCustomer->execute($customerData); }); if (!$customer->getId()) { throw new LocalizedException(__('Something went wrong during creating new customer account.')); } $output->writeln(''); $output->writeln('<info>User created with the following data:</info>'); $output->writeln('<comment>Customer ID: ' . $customer->getId()); $output->writeln('<comment>Customer Website ID ' . $customer->getWebsiteId()); $output->writeln('<comment>Customer First Name: ' . $customer->getFirstname()); $output->writeln('<comment>Customer Last Name: ' . $customer->getLastname()); $output->writeln(''); $output->writeln('<comment>Customer Email: ' . $customer->getEmail()); $output->writeln('<comment>Customer Password: ' . $input->getOption(NewCustomer::KEY_PASSWORD)); } /** * @return array */ private function getOptionsList() { return [ $this->getInput( NewCustomer::KEY_FIRSTNAME, null, InputOption::VALUE_REQUIRED, '(Required) Customer first name' ), $this->getInput( NewCustomer::KEY_LASTNAME, null, InputOption::VALUE_REQUIRED, '(Required) Customer last name' ), $this->getInput( NewCustomer::KEY_EMAIL, null, InputOption::VALUE_REQUIRED, '(Required) Customer email' ), $this->getInput( NewCustomer::KEY_PASSWORD, null, InputOption::VALUE_REQUIRED, '(Required) Customer password' ), $this->getInput( NewCustomer::KEY_WEBSITE, null, InputOption::VALUE_REQUIRED, '(Required) Website ID' ), ]; } /** * @param string $name * @param string|null $shortcut * @param integer|null $mode * @param string $description * @param string|null $default * @return InputOption */ private function getInput($name, $shortcut = null, $mode = null, $description = '', $default = null) { return $this->inputOptionFactory->create([ 'name' => $name, 'shortcut' => $shortcut, 'mode' => $mode, 'description' => $description, 'default' => $default ]); } }
html
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 // file app/code/Magently/Customer/Model/NewCustomer.php <?php namespace Magently\Customer\Model; use Magento\Customer\Api\Data\CustomerInterfaceFactory; use Magento\Customer\Api\AccountManagementInterface; use Magento\Framework\DataObject; /** * Class NewCustomer * The class responsible for creating new customer account */ class NewCustomer { /** * Customer First name key */ const KEY_FIRSTNAME = 'customer-firstname'; /** * Customer Last name key */ const KEY_LASTNAME = 'customer-lastname'; /** * Customer email key */ const KEY_EMAIL = 'customer-email'; /** * Customer password key */ const KEY_PASSWORD = 'customer-password'; /** * Customer website key */ const KEY_WEBSITE = 'customer-website'; /** * @var CustomerInterfaceFactory */ private $customerFactory; /** * @var AccountManagementInterface */ private $accountManagement; /** * NewCustomer constructor. * @param CustomerInterfaceFactory $customerFactory * @param AccountManagementInterface $accountManagement */ public function __construct( CustomerInterfaceFactory $customerFactory, AccountManagementInterface $accountManagement ) { $this->customerFactory = $customerFactory; $this->accountManagement = $accountManagement; } /** * @param DataObject $customerData * @return \Magento\Customer\Api\Data\CustomerInterface * @throws \Magento\Framework\Exception\LocalizedException If something went wrong. */ public function execute(DataObject $customerData) { $customer = $this->customerFactory->create() ->setFirstname($customerData->getData(self::KEY_FIRSTNAME)) ->setLastname($customerData->getData(self::KEY_LASTNAME)) ->setEmail($customerData->getData(self::KEY_EMAIL)) ->setWebsiteId($customerData->getData(self::KEY_WEBSITE)); return $this->accountManagement->createAccount( $customer, $customerData->getData(self::KEY_PASSWORD) ); } }

Let’s begin with the command class. First of all, instead of employing the direct creation of InputOption objects (which isn’t necessarily wrong in this particular case), I chose to make it using Factory. Instead of hardcoding area code, I chose to emulate it. And I’ve done it in the command class, not in the logic class so that it’s still possible to use the logic somewhere else.

Now for the logic class. I didn’t require a specific Input object and I made use of DataObject instead. It’s a fundamental Magento class that is a base for model/collections/and a whole lot of other classes. It gives us more versatility when it comes to passing different objects that simply extend this class. To create customer accounts, I used AccountManagementInterface, so I’m using what Magento is using when creating those accounts.

Using the command looks like this:

bash
1 php bin/magento customer:account:create --customer-firstname John --customer-lastname Doe --customer-email johndoe@example.com --customer-password customer123! --customer-website 1

Summary of my Magento code review

It’s worth mentioning that this code could be optimized even further. To make it easy to change an implementation you use, you could base it on the interface in the class responsible for creating customer accounts. You could use additional validators introduced in the command line. In the command class, you could use the interact() method to make it possible to input data on the fly. You could add a flag to display the customer’s password after it’s creating (show/hide). However, my aim in the original article was to show you how do Magento commands work, and creating a customer’s account was only used as an example.

That’s it! Feel free to comment and suggest your ideas for making it better - there’s always something new to learn.

And I can’t wait to update this article again when I read it in 3 years :)