From 25242578e6d2ade26c9c1680f321500ea3e18a06 Mon Sep 17 00:00:00 2001 From: Leon Helmus Date: Wed, 8 Mar 2023 17:53:48 +0100 Subject: [PATCH] Use constructor property promotion in module TaxImportExport Replace allmost all properties with constructor property promotion in module Theme: https://stitcher.io/blog/constructor-promotion-in-php-8 * Readable code * Make Magento less complex by removing properties which take up a lot of lines. * Imported all classes to make code more readable. I think the code would be a lot cleaner if all modules start using constructor promotions, since of 2.4.6 php 7.4 is dropped we can now make use of it. So let's start on it right now :). --- .../Adminhtml/Rate/Grid/Renderer/Country.php | 9 ++- .../Block/Adminhtml/Rate/ImportExport.php | 9 ++- .../Adminhtml/Rate/ImportExportHeader.php | 4 +- .../Controller/Adminhtml/Rate.php | 20 +++---- .../Controller/Adminhtml/Rate/ExportCsv.php | 6 +- .../Controller/Adminhtml/Rate/ExportPost.php | 15 +++-- .../Controller/Adminhtml/Rate/ExportXml.php | 6 +- .../Adminhtml/Rate/ImportExport.php | 14 +++-- .../Controller/Adminhtml/Rate/ImportPost.php | 19 +++--- .../Model/Rate/CsvImportHandler.php | 58 +++++++++---------- .../Magento/TaxImportExport/registration.php | 6 +- 11 files changed, 97 insertions(+), 69 deletions(-) diff --git a/app/code/Magento/TaxImportExport/Block/Adminhtml/Rate/Grid/Renderer/Country.php b/app/code/Magento/TaxImportExport/Block/Adminhtml/Rate/Grid/Renderer/Country.php index 3a560ac7c1093..15662bcc2be7e 100644 --- a/app/code/Magento/TaxImportExport/Block/Adminhtml/Rate/Grid/Renderer/Country.php +++ b/app/code/Magento/TaxImportExport/Block/Adminhtml/Rate/Grid/Renderer/Country.php @@ -9,15 +9,18 @@ */ namespace Magento\TaxImportExport\Block\Adminhtml\Rate\Grid\Renderer; -class Country extends \Magento\Backend\Block\Widget\Grid\Column\Renderer\Country +use Magento\Backend\Block\Widget\Grid\Column\Renderer\Country as ColumnRendererCountry; +use Magento\Framework\DataObject; + +class Country extends ColumnRendererCountry { /** * Render column for export * - * @param \Magento\Framework\DataObject $row + * @param DataObject $row * @return string */ - public function renderExport(\Magento\Framework\DataObject $row) + public function renderExport(DataObject $row) { return $row->getData($this->getColumn()->getIndex()); } diff --git a/app/code/Magento/TaxImportExport/Block/Adminhtml/Rate/ImportExport.php b/app/code/Magento/TaxImportExport/Block/Adminhtml/Rate/ImportExport.php index ab64567f4fe28..d629c7f953406 100644 --- a/app/code/Magento/TaxImportExport/Block/Adminhtml/Rate/ImportExport.php +++ b/app/code/Magento/TaxImportExport/Block/Adminhtml/Rate/ImportExport.php @@ -5,11 +5,14 @@ */ namespace Magento\TaxImportExport\Block\Adminhtml\Rate; +use Magento\Backend\Block\Template\Context; +use Magento\Backend\Block\Widget; + /** * @api * @since 100.0.2 */ -class ImportExport extends \Magento\Backend\Block\Widget +class ImportExport extends Widget { /** * @var string @@ -17,10 +20,10 @@ class ImportExport extends \Magento\Backend\Block\Widget protected $_template = 'Magento_TaxImportExport::importExport.phtml'; /** - * @param \Magento\Backend\Block\Template\Context $context + * @param Context $context * @param array $data */ - public function __construct(\Magento\Backend\Block\Template\Context $context, array $data = []) + public function __construct(Context $context, array $data = []) { parent::__construct($context, $data); $this->setUseContainer(true); diff --git a/app/code/Magento/TaxImportExport/Block/Adminhtml/Rate/ImportExportHeader.php b/app/code/Magento/TaxImportExport/Block/Adminhtml/Rate/ImportExportHeader.php index e223adc3adb1a..55736943aebc3 100644 --- a/app/code/Magento/TaxImportExport/Block/Adminhtml/Rate/ImportExportHeader.php +++ b/app/code/Magento/TaxImportExport/Block/Adminhtml/Rate/ImportExportHeader.php @@ -9,7 +9,9 @@ */ namespace Magento\TaxImportExport\Block\Adminhtml\Rate; -class ImportExportHeader extends \Magento\Backend\Block\Widget +use Magento\Backend\Block\Widget; + +class ImportExportHeader extends Widget { /** * Block's template diff --git a/app/code/Magento/TaxImportExport/Controller/Adminhtml/Rate.php b/app/code/Magento/TaxImportExport/Controller/Adminhtml/Rate.php index 008689b9ac6c7..c022a350016ff 100644 --- a/app/code/Magento/TaxImportExport/Controller/Adminhtml/Rate.php +++ b/app/code/Magento/TaxImportExport/Controller/Adminhtml/Rate.php @@ -5,10 +5,14 @@ */ namespace Magento\TaxImportExport\Controller\Adminhtml; +use Magento\Backend\App\Action; +use Magento\Backend\App\Action\Context; +use Magento\Framework\App\Response\Http\FileFactory; + /** * Adminhtml tax rate controller */ -abstract class Rate extends \Magento\Backend\App\Action +abstract class Rate extends Action { /** * Authorization level of a basic admin session @@ -18,19 +22,13 @@ abstract class Rate extends \Magento\Backend\App\Action const ADMIN_RESOURCE = 'Magento_Tax::manage_tax'; /** - * @var \Magento\Framework\App\Response\Http\FileFactory - */ - protected $fileFactory; - - /** - * @param \Magento\Backend\App\Action\Context $context - * @param \Magento\Framework\App\Response\Http\FileFactory $fileFactory + * @param Context $context + * @param FileFactory $fileFactory */ public function __construct( - \Magento\Backend\App\Action\Context $context, - \Magento\Framework\App\Response\Http\FileFactory $fileFactory + Context $context, + protected readonly FileFactory $fileFactory ) { - $this->fileFactory = $fileFactory; parent::__construct($context); } } diff --git a/app/code/Magento/TaxImportExport/Controller/Adminhtml/Rate/ExportCsv.php b/app/code/Magento/TaxImportExport/Controller/Adminhtml/Rate/ExportCsv.php index 958274c555b13..cc10a69879e1a 100644 --- a/app/code/Magento/TaxImportExport/Controller/Adminhtml/Rate/ExportCsv.php +++ b/app/code/Magento/TaxImportExport/Controller/Adminhtml/Rate/ExportCsv.php @@ -8,8 +8,10 @@ use Magento\Framework\App\ResponseInterface; use Magento\Framework\App\Filesystem\DirectoryList; use Magento\Framework\Controller\ResultFactory; +use Magento\Framework\View\Result\Layout; +use Magento\TaxImportExport\Controller\Adminhtml\Rate; -class ExportCsv extends \Magento\TaxImportExport\Controller\Adminhtml\Rate +class ExportCsv extends Rate { /** * Export rates grid to CSV format @@ -18,7 +20,7 @@ class ExportCsv extends \Magento\TaxImportExport\Controller\Adminhtml\Rate */ public function execute() { - /** @var \Magento\Framework\View\Result\Layout $resultLayout */ + /** @var Layout $resultLayout */ $resultLayout = $this->resultFactory->create(ResultFactory::TYPE_LAYOUT); $content = $resultLayout->getLayout()->getChildBlock('adminhtml.tax.rate.grid', 'grid.export'); diff --git a/app/code/Magento/TaxImportExport/Controller/Adminhtml/Rate/ExportPost.php b/app/code/Magento/TaxImportExport/Controller/Adminhtml/Rate/ExportPost.php index f23fe8ffae7ae..4598e7de0092c 100644 --- a/app/code/Magento/TaxImportExport/Controller/Adminhtml/Rate/ExportPost.php +++ b/app/code/Magento/TaxImportExport/Controller/Adminhtml/Rate/ExportPost.php @@ -7,8 +7,13 @@ use Magento\Framework\App\Filesystem\DirectoryList; use Magento\Framework\App\ResponseInterface; +use Magento\Framework\DataObject; +use Magento\Store\Model\Store; +use Magento\Tax\Model\Calculation\Rate\Title; +use Magento\Tax\Model\ResourceModel\Calculation\Rate\Collection; +use Magento\TaxImportExport\Controller\Adminhtml\Rate; -class ExportPost extends \Magento\TaxImportExport\Controller\Adminhtml\Rate +class ExportPost extends Rate { /** * Export action from import/export tax @@ -18,7 +23,7 @@ class ExportPost extends \Magento\TaxImportExport\Controller\Adminhtml\Rate public function execute() { /** start csv content and set template */ - $headers = new \Magento\Framework\DataObject( + $headers = new DataObject( [ 'code' => __('Code'), 'country_name' => __('Country'), @@ -38,7 +43,7 @@ public function execute() $taxCalculationRateTitleDict = []; foreach ($this->_objectManager->create( - \Magento\Store\Model\Store::class + Store::class )->getCollection()->setLoadDefault( false ) as $store) { @@ -52,7 +57,7 @@ public function execute() $content .= "\n"; foreach ($this->_objectManager->create( - \Magento\Tax\Model\Calculation\Rate\Title::class + Title::class )->getCollection() as $title) { $rateId = $title->getTaxCalculationRateId(); @@ -65,7 +70,7 @@ public function execute() unset($title); $collection = $this->_objectManager->create( - \Magento\Tax\Model\ResourceModel\Calculation\Rate\Collection::class + Collection::class )->joinCountryTable()->joinRegionTable(); while ($rate = $collection->fetchItem()) { diff --git a/app/code/Magento/TaxImportExport/Controller/Adminhtml/Rate/ExportXml.php b/app/code/Magento/TaxImportExport/Controller/Adminhtml/Rate/ExportXml.php index e31f0fa0723f1..5d02b61ba7159 100644 --- a/app/code/Magento/TaxImportExport/Controller/Adminhtml/Rate/ExportXml.php +++ b/app/code/Magento/TaxImportExport/Controller/Adminhtml/Rate/ExportXml.php @@ -8,8 +8,10 @@ use Magento\Framework\App\ResponseInterface; use Magento\Framework\App\Filesystem\DirectoryList; use Magento\Framework\Controller\ResultFactory; +use Magento\Framework\View\Result\Layout; +use Magento\TaxImportExport\Controller\Adminhtml\Rate; -class ExportXml extends \Magento\TaxImportExport\Controller\Adminhtml\Rate +class ExportXml extends Rate { /** * Export rates grid to XML format @@ -18,7 +20,7 @@ class ExportXml extends \Magento\TaxImportExport\Controller\Adminhtml\Rate */ public function execute() { - /** @var \Magento\Framework\View\Result\Layout $resultLayout */ + /** @var Layout $resultLayout */ $resultLayout = $this->resultFactory->create(ResultFactory::TYPE_LAYOUT); $content = $resultLayout->getLayout()->getChildBlock('adminhtml.tax.rate.grid', 'grid.export'); diff --git a/app/code/Magento/TaxImportExport/Controller/Adminhtml/Rate/ImportExport.php b/app/code/Magento/TaxImportExport/Controller/Adminhtml/Rate/ImportExport.php index 4264f5c3b7765..82192b200cb2c 100644 --- a/app/code/Magento/TaxImportExport/Controller/Adminhtml/Rate/ImportExport.php +++ b/app/code/Magento/TaxImportExport/Controller/Adminhtml/Rate/ImportExport.php @@ -5,10 +5,14 @@ */ namespace Magento\TaxImportExport\Controller\Adminhtml\Rate; +use Magento\Backend\Model\View\Result\Page; use Magento\Framework\App\Action\HttpGetActionInterface as HttpGetActionInterface; use Magento\Framework\Controller\ResultFactory; +use Magento\TaxImportExport\Block\Adminhtml\Rate\ImportExport as RateImportExport; +use Magento\TaxImportExport\Block\Adminhtml\Rate\ImportExportHeader; +use Magento\TaxImportExport\Controller\Adminhtml\Rate; -class ImportExport extends \Magento\TaxImportExport\Controller\Adminhtml\Rate implements HttpGetActionInterface +class ImportExport extends Rate implements HttpGetActionInterface { /** * Authorization level of a basic admin session @@ -20,21 +24,21 @@ class ImportExport extends \Magento\TaxImportExport\Controller\Adminhtml\Rate im /** * Import and export Page * - * @return \Magento\Backend\Model\View\Result\Page + * @return Page */ public function execute() { - /** @var \Magento\Backend\Model\View\Result\Page $resultPage */ + /** @var Page $resultPage */ $resultPage = $this->resultFactory->create(ResultFactory::TYPE_PAGE); $resultPage->setActiveMenu('Magento_TaxImportExport::system_convert_tax'); $resultPage->addContent( $resultPage->getLayout()->createBlock( - \Magento\TaxImportExport\Block\Adminhtml\Rate\ImportExportHeader::class + ImportExportHeader::class ) ); $resultPage->addContent( - $resultPage->getLayout()->createBlock(\Magento\TaxImportExport\Block\Adminhtml\Rate\ImportExport::class) + $resultPage->getLayout()->createBlock(RateImportExport::class) ); $resultPage->getConfig()->getTitle()->prepend(__('Tax Zones and Rates')); $resultPage->getConfig()->getTitle()->prepend(__('Import and Export Tax Rates')); diff --git a/app/code/Magento/TaxImportExport/Controller/Adminhtml/Rate/ImportPost.php b/app/code/Magento/TaxImportExport/Controller/Adminhtml/Rate/ImportPost.php index ffdb7d3782df1..eb7ae2c48941d 100644 --- a/app/code/Magento/TaxImportExport/Controller/Adminhtml/Rate/ImportPost.php +++ b/app/code/Magento/TaxImportExport/Controller/Adminhtml/Rate/ImportPost.php @@ -5,36 +5,41 @@ */ namespace Magento\TaxImportExport\Controller\Adminhtml\Rate; +use Exception; +use Magento\Backend\Model\View\Result\Redirect as ResultRedirect; use Magento\Framework\Controller\ResultFactory; +use Magento\Framework\Exception\LocalizedException; +use Magento\TaxImportExport\Controller\Adminhtml\Rate; +use Magento\TaxImportExport\Model\Rate\CsvImportHandler; -class ImportPost extends \Magento\TaxImportExport\Controller\Adminhtml\Rate +class ImportPost extends Rate { /** * import action from import/export tax * - * @return \Magento\Backend\Model\View\Result\Redirect + * @return ResultRedirect */ public function execute() { $importRatesFile = $this->getRequest()->getFiles('import_rates_file'); if ($this->getRequest()->isPost() && isset($importRatesFile['tmp_name'])) { try { - /** @var $importHandler \Magento\TaxImportExport\Model\Rate\CsvImportHandler */ + /** @var CsvImportHandler $importHandler */ $importHandler = $this->_objectManager->create( - \Magento\TaxImportExport\Model\Rate\CsvImportHandler::class + CsvImportHandler::class ); $importHandler->importFromCsvFile($importRatesFile); $this->messageManager->addSuccess(__('The tax rate has been imported.')); - } catch (\Magento\Framework\Exception\LocalizedException $e) { + } catch (LocalizedException $e) { $this->messageManager->addError($e->getMessage()); - } catch (\Exception $e) { + } catch (Exception $e) { $this->messageManager->addError(__('Invalid file upload attempt')); } } else { $this->messageManager->addError(__('Invalid file upload attempt')); } - /** @var \Magento\Backend\Model\View\Result\Redirect $resultRedirect */ + /** @var ResultRedirect $resultRedirect */ $resultRedirect = $this->resultFactory->create(ResultFactory::TYPE_REDIRECT); $resultRedirect->setUrl($this->_redirect->getRedirectUrl()); return $resultRedirect; diff --git a/app/code/Magento/TaxImportExport/Model/Rate/CsvImportHandler.php b/app/code/Magento/TaxImportExport/Model/Rate/CsvImportHandler.php index c8cba30c9cd25..87632b2391ff8 100644 --- a/app/code/Magento/TaxImportExport/Model/Rate/CsvImportHandler.php +++ b/app/code/Magento/TaxImportExport/Model/Rate/CsvImportHandler.php @@ -5,6 +5,14 @@ */ namespace Magento\TaxImportExport\Model\Rate; +use Magento\Directory\Model\CountryFactory; +use Magento\Directory\Model\ResourceModel\Region\Collection as RegionCollection; +use Magento\Framework\Exception\LocalizedException; +use Magento\Framework\File\Csv as FileCsv; +use Magento\Store\Model\ResourceModel\Store\Collection as StoreCollection; +use Magento\Tax\Model\Calculation\Rate as CalculationRate; +use Magento\Tax\Model\Calculation\RateFactory; + /** * Tax Rate CSV Import Handler * @@ -16,7 +24,7 @@ class CsvImportHandler /** * Collection of publicly available stores * - * @var \Magento\Store\Model\ResourceModel\Store\Collection + * @var StoreCollection */ protected $_publicStores; @@ -25,51 +33,43 @@ class CsvImportHandler * * The instance is used to retrieve regions based on country code * - * @var \Magento\Directory\Model\ResourceModel\Region\Collection + * @var RegionCollection */ protected $_regionCollection; /** * Country factory * - * @var \Magento\Directory\Model\CountryFactory + * @var CountryFactory */ protected $_countryFactory; /** * Tax rate factory * - * @var \Magento\Tax\Model\Calculation\RateFactory + * @var RateFactory */ protected $_taxRateFactory; /** - * CSV Processor - * - * @var \Magento\Framework\File\Csv - */ - protected $csvProcessor; - - /** - * @param \Magento\Store\Model\ResourceModel\Store\Collection $storeCollection - * @param \Magento\Directory\Model\ResourceModel\Region\Collection $regionCollection - * @param \Magento\Directory\Model\CountryFactory $countryFactory - * @param \Magento\Tax\Model\Calculation\RateFactory $taxRateFactory - * @param \Magento\Framework\File\Csv $csvProcessor + * @param StoreCollection $storeCollection + * @param RegionCollection $regionCollection + * @param CountryFactory $countryFactory + * @param RateFactory $taxRateFactory + * @param FileCsv $csvProcessor CSV Processor */ public function __construct( - \Magento\Store\Model\ResourceModel\Store\Collection $storeCollection, - \Magento\Directory\Model\ResourceModel\Region\Collection $regionCollection, - \Magento\Directory\Model\CountryFactory $countryFactory, - \Magento\Tax\Model\Calculation\RateFactory $taxRateFactory, - \Magento\Framework\File\Csv $csvProcessor + StoreCollection $storeCollection, + RegionCollection $regionCollection, + CountryFactory $countryFactory, + RateFactory $taxRateFactory, + protected readonly FileCsv $csvProcessor ) { // prevent admin store from loading $this->_publicStores = $storeCollection->setLoadDefault(false); $this->_regionCollection = $regionCollection; $this->_countryFactory = $countryFactory; $this->_taxRateFactory = $taxRateFactory; - $this->csvProcessor = $csvProcessor; } /** @@ -97,12 +97,12 @@ public function getRequiredCsvFields() * * @param array $file file info retrieved from $_FILES array * @return void - * @throws \Magento\Framework\Exception\LocalizedException + * @throws LocalizedException */ public function importFromCsvFile($file) { if (!isset($file['tmp_name'])) { - throw new \Magento\Framework\Exception\LocalizedException(__('Invalid file upload attempt.')); + throw new LocalizedException(__('Invalid file upload attempt.')); } $ratesRawData = $this->csvProcessor->getData($file['tmp_name']); // first row of file represents headers @@ -153,7 +153,7 @@ protected function _filterFileFields(array $fileFields) * @param array $invalidFields assoc array of invalid file fields * @param array $validFields assoc array of valid file fields * @return array - * @throws \Magento\Framework\Exception\LocalizedException + * @throws LocalizedException * @SuppressWarnings(PHPMD.UnusedLocalVariable) */ protected function _filterRateData(array $rateRawData, array $invalidFields, array $validFields) @@ -173,7 +173,7 @@ protected function _filterRateData(array $rateRawData, array $invalidFields, arr } // check if number of fields in row match with number of valid fields if (count($rateRawData[$rowIndex]) != $validFieldsNum) { - throw new \Magento\Framework\Exception\LocalizedException(__('Invalid file format.')); + throw new LocalizedException(__('Invalid file format.')); } } return $rateRawData; @@ -229,7 +229,7 @@ protected function _isStoreCodeValid($storeCode) * @param array $regionsCache cache of regions of already used countries (is used to optimize performance) * @param array $storesCache cache of stores related to tax rate titles * @return array regions cache populated with regions related to country of imported tax rate - * @throws \Magento\Framework\Exception\LocalizedException + * @throws LocalizedException */ protected function _importRate(array $rateData, array $regionsCache, array $storesCache) { @@ -237,7 +237,7 @@ protected function _importRate(array $rateData, array $regionsCache, array $stor $countryCode = $rateData[1]; $country = $this->_countryFactory->create()->loadByCode($countryCode, 'iso2_code'); if (!$country->getId()) { - throw new \Magento\Framework\Exception\LocalizedException(__('Country code is invalid: %1', $countryCode)); + throw new LocalizedException(__('Country code is invalid: %1', $countryCode)); } $regionsCache = $this->_addCountryRegionsToCache($countryCode, $regionsCache); @@ -259,7 +259,7 @@ protected function _importRate(array $rateData, array $regionsCache, array $stor ]; // try to load existing rate - /** @var $rateModel \Magento\Tax\Model\Calculation\Rate */ + /** @var CalculationRate $rateModel */ $rateModel = $this->_taxRateFactory->create()->loadByCode($modelData['code']); $rateModel->addData($modelData); diff --git a/app/code/Magento/TaxImportExport/registration.php b/app/code/Magento/TaxImportExport/registration.php index 67b67b55cbeee..ac4ab524c5a8a 100644 --- a/app/code/Magento/TaxImportExport/registration.php +++ b/app/code/Magento/TaxImportExport/registration.php @@ -6,4 +6,8 @@ use Magento\Framework\Component\ComponentRegistrar; -ComponentRegistrar::register(ComponentRegistrar::MODULE, 'Magento_TaxImportExport', __DIR__); +ComponentRegistrar::register( + ComponentRegistrar::MODULE, + 'Magento_TaxImportExport', + __DIR__ +);