From 1f0b6b047d3d6f861c8c7ae6f63dd7ee3db3ea46 Mon Sep 17 00:00:00 2001 From: Rick Steckles Date: Wed, 25 Nov 2020 22:00:15 +0000 Subject: [PATCH 01/18] Use the content type instead of file extension when parsing remote sources --- Model/Processor.php | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/Model/Processor.php b/Model/Processor.php index d998516..ddb9076 100644 --- a/Model/Processor.php +++ b/Model/Processor.php @@ -433,6 +433,12 @@ private function getExtension($source) { // phpcs:ignore Magento2.Functions.DiscouragedFunction $extension = pathinfo($source, PATHINFO_EXTENSION); + + // For remote files, use the mime type to determine the extension + if ($this->isRemoteSource($source)) { + $extension = $this->getRemoteContentExtension($source); + } + if (strtolower($extension) === 'yaml') { return self::SOURCE_YAML; } @@ -457,6 +463,29 @@ private function getData($source) file_get_contents(BP . '/' . $source); // phpcs:ignore Magento2.Functions.DiscouragedFunction } + /** + * @param $source + * @return array|bool|false|float|int|mixed|string|null + * @throws \Exception + */ + private function getRemoteContentExtension($source) + { + try { + // phpcs:ignore Magento2.Functions.DiscouragedFunction + $streamContext = stream_context_create(['ssl' => ['verify_peer' => false, 'verify_peer_name' => false]]); + } catch (\Exception $e) { + return ''; + } + + // phpcs:ignore Magento2.Functions.DiscouragedFunction + $headers = get_headers($source, 1); + $contentType = array_key_exists('Content-Type', $headers) ? $headers['Content-Type'] : ''; + + // Parse the 'extension' from the content type + preg_match('%^text/([a-z]+)%', $contentType, $matches); + return (count($matches) == 2) ? $matches[1] : null; + } + /** * @param $source * @return array|bool|float|int|mixed|string|null From f283cf3861db41eda777659298596d093ba12b3f Mon Sep 17 00:00:00 2001 From: Rick Steckles Date: Thu, 26 Nov 2020 08:57:35 +0000 Subject: [PATCH 02/18] Standardise indentation --- Model/Processor.php | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/Model/Processor.php b/Model/Processor.php index ddb9076..2331aec 100644 --- a/Model/Processor.php +++ b/Model/Processor.php @@ -436,7 +436,7 @@ private function getExtension($source) // For remote files, use the mime type to determine the extension if ($this->isRemoteSource($source)) { - $extension = $this->getRemoteContentExtension($source); + $extension = $this->getRemoteContentExtension($source); } if (strtolower($extension) === 'yaml') { @@ -470,20 +470,20 @@ private function getData($source) */ private function getRemoteContentExtension($source) { - try { - // phpcs:ignore Magento2.Functions.DiscouragedFunction - $streamContext = stream_context_create(['ssl' => ['verify_peer' => false, 'verify_peer_name' => false]]); - } catch (\Exception $e) { - return ''; - } - - // phpcs:ignore Magento2.Functions.DiscouragedFunction - $headers = get_headers($source, 1); - $contentType = array_key_exists('Content-Type', $headers) ? $headers['Content-Type'] : ''; - - // Parse the 'extension' from the content type - preg_match('%^text/([a-z]+)%', $contentType, $matches); - return (count($matches) == 2) ? $matches[1] : null; + try { + // phpcs:ignore Magento2.Functions.DiscouragedFunction + $streamContext = stream_context_create(['ssl' => ['verify_peer' => false, 'verify_peer_name' => false]]); + } catch (\Exception $e) { + return ''; + } + + // phpcs:ignore Magento2.Functions.DiscouragedFunction + $headers = get_headers($source, 1); + $contentType = array_key_exists('Content-Type', $headers) ? $headers['Content-Type'] : ''; + + // Parse the 'extension' from the content type + preg_match('%^text/([a-z]+)%', $contentType, $matches); + return (count($matches) == 2) ? $matches[1] : null; } /** From be4712cc0d32da67815ba4120b67491a1ab6f916 Mon Sep 17 00:00:00 2001 From: Rick Steckles Date: Thu, 26 Nov 2020 10:27:57 +0000 Subject: [PATCH 03/18] Use fgetcsv to parse CSV input, catering for new lines in fields --- Model/Processor.php | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/Model/Processor.php b/Model/Processor.php index 2331aec..3ac57fb 100644 --- a/Model/Processor.php +++ b/Model/Processor.php @@ -383,14 +383,17 @@ private function parseData($source, $sourceType) { if ($this->canParseAndProcess($source) === true) { $ext = ($sourceType !== null) ? $sourceType : $this->getExtension($source); - $sourceData = $this->getData($source); + if ($ext === self::SOURCE_YAML) { + $sourceData = $this->getData($source); return $this->parseYamlData($sourceData); } if ($ext === self::SOURCE_CSV) { - return $this->parseCsvData($sourceData); + // Data is read directly from the source by parseCsvData() + return $this->parseCsvData($source); } if ($ext === self::SOURCE_JSON) { + $sourceData = $this->getData($source); return $this->parseJsonData($sourceData); } } @@ -435,7 +438,7 @@ private function getExtension($source) $extension = pathinfo($source, PATHINFO_EXTENSION); // For remote files, use the mime type to determine the extension - if ($this->isRemoteSource($source)) { + if ($this->isSourceRemote($source)) { $extension = $this->getRemoteContentExtension($source); } @@ -520,17 +523,33 @@ private function parseYamlData($source) */ private function parseCsvData($source) { - $lines = explode("\n", $source); - $headerRow = str_getcsv(array_shift($lines)); + // Get a handle to the source data, whether it's remote or local + $remoteSource = $this->isSourceRemote($source); + + if ($remoteSource) { + // phpcs:ignore Magento2.Functions.DiscouragedFunction + $streamContext = stream_context_create(['ssl' => ['verify_peer' => false, 'verify_peer_name' => false]]); + $handle = fopen($source, 'r', false, $streamContext); + } + else { + $handle = fopen($source, 'r'); + } + + // Read the header row + $headerRow = fgetcsv($handle); $csvData = [$headerRow]; - foreach ($lines as $line) { - $csvLine = str_getcsv($line); + + // Read all other rows and build up an array, with row headers as keys + while (($csvLine = fgetcsv($handle)) !== FALSE) { $csvRow = []; + foreach (array_keys($headerRow) as $key) { $csvRow[$key] = (array_key_exists($key, $csvLine) === true) ? $csvLine[$key] : ''; } + $csvData[] = $csvRow; } + return $csvData; } From a29d436a27e987186ae5d57c9287a62f43ecf07a Mon Sep 17 00:00:00 2001 From: Rick Steckles Date: Thu, 26 Nov 2020 12:24:15 +0000 Subject: [PATCH 04/18] Remove unnecessary variable declaration --- Model/Processor.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Model/Processor.php b/Model/Processor.php index 3ac57fb..65a3118 100644 --- a/Model/Processor.php +++ b/Model/Processor.php @@ -524,9 +524,7 @@ private function parseYamlData($source) private function parseCsvData($source) { // Get a handle to the source data, whether it's remote or local - $remoteSource = $this->isSourceRemote($source); - - if ($remoteSource) { + if ($this->isSourceRemote($source)) { // phpcs:ignore Magento2.Functions.DiscouragedFunction $streamContext = stream_context_create(['ssl' => ['verify_peer' => false, 'verify_peer_name' => false]]); $handle = fopen($source, 'r', false, $streamContext); From 1dacf9e65b9ca4ee301afebda06571410ca9ecbc Mon Sep 17 00:00:00 2001 From: Rick Steckles Date: Thu, 26 Nov 2020 12:28:24 +0000 Subject: [PATCH 05/18] - Close file after reading --- Model/Processor.php | 1 + 1 file changed, 1 insertion(+) diff --git a/Model/Processor.php b/Model/Processor.php index 65a3118..deb10d5 100644 --- a/Model/Processor.php +++ b/Model/Processor.php @@ -548,6 +548,7 @@ private function parseCsvData($source) $csvData[] = $csvRow; } + fclose($handle); return $csvData; } From d7b3cb2ec9d1e53803dba0aaa12ceb4177aaab71 Mon Sep 17 00:00:00 2001 From: Rick Steckles Date: Thu, 26 Nov 2020 17:37:45 +0000 Subject: [PATCH 06/18] Addressed various complaints from phpcs/phpmd --- Model/Processor.php | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/Model/Processor.php b/Model/Processor.php index deb10d5..302c0f2 100644 --- a/Model/Processor.php +++ b/Model/Processor.php @@ -7,6 +7,7 @@ use CtiDigital\Configurator\Api\ComponentListInterface; use CtiDigital\Configurator\Api\LoggerInterface; use CtiDigital\Configurator\Exception\ComponentException; +use Exception; use Symfony\Component\Yaml\Parser; use Magento\Framework\App\State; use Magento\Framework\App\Area; @@ -481,10 +482,11 @@ private function getRemoteContentExtension($source) } // phpcs:ignore Magento2.Functions.DiscouragedFunction - $headers = get_headers($source, 1); + $headers = get_headers($source, 1, $streamContext); $contentType = array_key_exists('Content-Type', $headers) ? $headers['Content-Type'] : ''; // Parse the 'extension' from the content type + $matches = []; preg_match('%^text/([a-z]+)%', $contentType, $matches); return (count($matches) == 2) ? $matches[1] : null; } @@ -525,20 +527,33 @@ private function parseCsvData($source) { // Get a handle to the source data, whether it's remote or local if ($this->isSourceRemote($source)) { + try { + // phpcs:ignore Magento2.Functions.DiscouragedFunction + $streamContext = stream_context_create(['ssl' => + [ + 'verify_peer' => false, + 'verify_peer_name' => false + ] + ]); + + // phpcs:ignore Magento2.Functions.DiscouragedFunction + $handle = fopen($source, 'r', false, $streamContext); + } catch (Exception $ex) { + throw new ComponentException("Can't open CSV source for reading: {$ex->getMessage()}"); + } + } else { // phpcs:ignore Magento2.Functions.DiscouragedFunction - $streamContext = stream_context_create(['ssl' => ['verify_peer' => false, 'verify_peer_name' => false]]); - $handle = fopen($source, 'r', false, $streamContext); - } - else { $handle = fopen($source, 'r'); } // Read the header row + // phpcs:ignore Magento2.Functions.DiscouragedFunction $headerRow = fgetcsv($handle); $csvData = [$headerRow]; // Read all other rows and build up an array, with row headers as keys - while (($csvLine = fgetcsv($handle)) !== FALSE) { + // phpcs:ignore Magento2.Functions.DiscouragedFunction + while (($csvLine = fgetcsv($handle)) !== false) { $csvRow = []; foreach (array_keys($headerRow) as $key) { @@ -548,6 +563,7 @@ private function parseCsvData($source) $csvData[] = $csvRow; } + // phpcs:ignore Magento2.Functions.DiscouragedFunction fclose($handle); return $csvData; } From c744c43eafaad55d0c7f0b1f2e0c87411a31035c Mon Sep 17 00:00:00 2001 From: Rick Steckles Date: Thu, 26 Nov 2020 20:17:21 +0000 Subject: [PATCH 07/18] One more change to satisfy phpmd... --- Model/Processor.php | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/Model/Processor.php b/Model/Processor.php index 302c0f2..c53120f 100644 --- a/Model/Processor.php +++ b/Model/Processor.php @@ -520,10 +520,9 @@ private function parseYamlData($source) /** * @param $source - * @return array - * @throws \Exception + * @return mixed */ - private function parseCsvData($source) + private function getFileHandle($source) { // Get a handle to the source data, whether it's remote or local if ($this->isSourceRemote($source)) { @@ -537,15 +536,25 @@ private function parseCsvData($source) ]); // phpcs:ignore Magento2.Functions.DiscouragedFunction - $handle = fopen($source, 'r', false, $streamContext); + return fopen($source, 'r', false, $streamContext); } catch (Exception $ex) { throw new ComponentException("Can't open CSV source for reading: {$ex->getMessage()}"); } - } else { - // phpcs:ignore Magento2.Functions.DiscouragedFunction - $handle = fopen($source, 'r'); } + // phpcs:ignore Magento2.Functions.DiscouragedFunction + return fopen($source, 'r'); + } + + /** + * @param $source + * @return array + * @throws \Exception + */ + private function parseCsvData($source) + { + $handle = $this->getFileHandle($source); + // Read the header row // phpcs:ignore Magento2.Functions.DiscouragedFunction $headerRow = fgetcsv($handle); From c8501bb2f27f2cd095001d107f771f9aa15b45ff Mon Sep 17 00:00:00 2001 From: Paul Partington Date: Fri, 4 Dec 2020 10:42:59 +0000 Subject: [PATCH 08/18] Add a Validator class for the product import. --- Component/Product/Validator.php | 206 ++++++++++++++++++++++++++++++++ Component/Products.php | 23 +++- 2 files changed, 227 insertions(+), 2 deletions(-) create mode 100644 Component/Product/Validator.php diff --git a/Component/Product/Validator.php b/Component/Product/Validator.php new file mode 100644 index 0000000..059c4f0 --- /dev/null +++ b/Component/Product/Validator.php @@ -0,0 +1,206 @@ +importAdapterFactory = $importAdapterFactory; + } + + /** + * @return array + */ + public function getLogs() + { + return $this->logs; + } + + /** + * @return array + */ + public function getRemovedRows() + { + return $this->removedRows; + } + + /** + * @param $rowData + * @param $row + * @param $attributeCode + * @param $errorMessage + * @param string $type + */ + private function writeLog($rowData, $row, $attributeCode, $errorMessage, $type = self::IMPORT_DATA_ACTION_REMOVE) + { + $sku = isset($rowData['sku']) ? $rowData['sku'] : null; + $productIdentifierMessage = ($sku !== null) ? sprintf('SKU: %s', $sku) : sprintf('Row Number : %s', $row); + switch ($type) { + case self::IMPORT_DATA_ACTION_NULLIFY: + $message = sprintf('%s Error: %s Resolution: Unset the value for attribute code %s', $productIdentifierMessage, $errorMessage, $attributeCode); + break; + default: + $message = sprintf('%s Error: %s Resolution: Removed the row due to error with attribute code %s', $productIdentifierMessage, $errorMessage, $attributeCode); + $this->removedRows[$row] = $message; + break; + } + $this->logs[] = $message; + } + + /** + * Runs the import data through the validation steps and returns the modified values + * + * @param Importer $import + * @param $importLines + * @return array + * @throws \Magento\Framework\Exception\LocalizedException + */ + public function getValidatedImport(Importer $import, $importLines) + { + $this->logs = []; + $failedImportRows = $this->getImportRowFailures($import, $importLines); + $importLines = $this->omitItemsFromImport($importLines, $failedImportRows); + return $importLines; + } + + /** + * @param Importer $import + * @param $importLines + * @return array + * @throws \Magento\Framework\Exception\LocalizedException + */ + public function getImportRowFailures(Importer $import, $importLines) + { + $failedRows = []; + // Creates a validation model and runs the import data through so we can find which rows would fail + $validation = $import->createImportModel(); + $validationSource = $this->importAdapterFactory->create([ + 'data' => $importLines, + 'multipleValueSeparator' => Products::SEPARATOR + ]); + $validation->validateSource($validationSource); + $errors = $validation->getErrorAggregator(); + foreach ($errors->getRowsGroupedByErrorCode() as $error => $rows) { + if (is_array($rows)) { + $failedRow = $this->formatRowRemoveData($error, $rows); + $failedRows[] = $failedRow; + } + } + return $failedRows; + } + + /** + * Either removes a row entirely or nulls specific attributes that we know are okay to ignore + * + * @param array $importLines + * @param array $failedRows + * @return array + */ + public function omitItemsFromImport(array $importLines, array $failedRows) + { + foreach ($failedRows as $failedRow) { + $attributeCode = $failedRow['attribute_code']; + foreach ($failedRow['rows'] as $row) { + switch ($failedRow['action']) { + case self::IMPORT_DATA_ACTION_NULLIFY: + if (isset($importLines[$row][$attributeCode])) { + $this->writeLog($importLines[$row], $row, $attributeCode, $failedRow['message'], self::IMPORT_DATA_ACTION_NULLIFY); + $importLines[$row][$attributeCode] = null; + } + break; + default: + if (isset($importLines[$row])) { + $this->writeLog($importLines[$row], $row, $attributeCode, $failedRow['message'], self::IMPORT_DATA_ACTION_REMOVE); + unset($importLines[$row]); + } + } + } + } + $importLines = array_values($importLines); + return $importLines; + } + + /** + * Gets the attribute code from the error returned by the validator + * + * @param $error + * @return string|null + */ + public function getAttributeCodeFromError($error) + { + $matches = []; + $attributeCode = null; + preg_match('/attribute\s([^\s]*)/', $error, $matches); + if (isset($matches[1])) { + $attributeCode = $matches[1]; + } + return $attributeCode; + } + + /** + * Processes the error into a set format + * + * @param $error + * @param $rows + * @return array + */ + private function formatRowRemoveData($error, $rows) + { + // Magento increases the row number by 1 as it assumes you've uploaded a CSV file with a header + $rowsProcessed = array_map(function ($row) { + return $row - 1; + }, $rows); + + $attributeCode = $this->getAttributeCodeFromError($error); + $action = (in_array($attributeCode, self::ATTRIBUTES_NULLIFY_ALLOW_LIST)) ? self::IMPORT_DATA_ACTION_NULLIFY : self::IMPORT_DATA_ACTION_REMOVE; + + return [ + 'action' => $action, + 'message' => $error, + 'rows' => $rowsProcessed, + 'attribute_code' => $attributeCode + ]; + } +} diff --git a/Component/Products.php b/Component/Products.php index 7d79ceb..503ece3 100644 --- a/Component/Products.php +++ b/Component/Products.php @@ -8,6 +8,8 @@ use CtiDigital\Configurator\Component\Product\AttributeOption; use FireGento\FastSimpleImport\Model\ImporterFactory; use CtiDigital\Configurator\Exception\ComponentException; +use CtiDigital\Configurator\Component\Product\ValidatorFactory; +use CtiDigital\Configurator\Component\Product\Validator; /** * @SuppressWarnings(PHPMD.CouplingBetweenObjects) @@ -69,6 +71,11 @@ class Products implements ComponentInterface */ protected $image; + /** + * @var ValidatorFactory + */ + protected $validatorFactory; + /** * @var AttributeOption */ @@ -99,6 +106,7 @@ class Products implements ComponentInterface * @param ImporterFactory $importerFactory * @param ProductFactory $productFactory * @param Image $image + * @param ValidatorFactory $validatorFactory * @param AttributeOption $attributeOption * @param LoggerInterface $log */ @@ -106,12 +114,14 @@ public function __construct( ImporterFactory $importerFactory, ProductFactory $productFactory, Image $image, + ValidatorFactory $validatorFactory, AttributeOption $attributeOption, LoggerInterface $log ) { $this->productFactory= $productFactory; $this->importerFactory = $importerFactory; $this->image = $image; + $this->validatorFactory = $validatorFactory; $this->attributeOption = $attributeOption; $this->log = $log; } @@ -176,11 +186,20 @@ public function execute($data = null) ); } $this->attributeOption->saveOptions(); - $this->log->logInfo(sprintf('Attempting to import %s rows', count($this->successProducts))); + $this->log->logInfo('Validating import...'); + $validatorImport = $this->importerFactory->create(); + $validatorImport->setMultipleValueSeparator(self::SEPARATOR); + /** + * @var Validator $validatorModel + */ + $validatorModel = $this->validatorFactory->create(); + $validatedProductsArray = $validatorModel->getValidatedImport($validatorImport, $productsArray); + $this->log->logInfo(sprintf('Removed %s products after validation.', count($validatorModel->getRemovedRows()))); + $this->log->logInfo(sprintf('Attempting to import %s rows', count($validatedProductsArray))); try { $import = $this->importerFactory->create(); $import->setMultipleValueSeparator(self::SEPARATOR); - $import->processImport($productsArray); + $import->processImport($validatedProductsArray); } catch (\Exception $e) { $this->log->logError($e->getMessage()); } From 13f2ed3c57c795f0595f02a582a9310c784b5277 Mon Sep 17 00:00:00 2001 From: Paul Partington Date: Mon, 7 Dec 2020 17:57:05 +0000 Subject: [PATCH 09/18] Update how the variation separator is added to prevent it being included without a product. --- Component/Products.php | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/Component/Products.php b/Component/Products.php index 503ece3..4568513 100644 --- a/Component/Products.php +++ b/Component/Products.php @@ -165,9 +165,11 @@ public function execute($data = null) } if ($this->isConfigurable($productArray)) { $variations = $this->constructConfigurableVariations($productArray); - if (strlen($variations) > 0) { - $productArray['configurable_variations'] = $variations; + if (strlen($variations) === 0) { + $this->skippedProducts[] = $product[$this->skuColumn]; + continue; } + $productArray['configurable_variations'] = $variations; unset($productArray['associated_products']); unset($productArray['configurable_attributes']); } @@ -273,6 +275,9 @@ public function constructConfigurableVariations($data) $productsCount = count($products); $count = 0; foreach ($products as $sku) { + if ($count > $productsCount) { + $variations .= '|'; + } $productModel = $this->productFactory->create(); $id = $productModel->getIdBySku($sku); $productModel->load($id); @@ -283,9 +288,6 @@ public function constructConfigurableVariations($data) $variations .= 'sku=' . $sku . self::SEPARATOR . $configSkuAttributes; } $count++; - if ($count < $productsCount) { - $variations .= '|'; - } } } } From 1970e0bb2a59c9e0c77057aa9e02f9c496960720 Mon Sep 17 00:00:00 2001 From: cesamuel Date: Thu, 10 Dec 2020 10:32:29 +0000 Subject: [PATCH 10/18] Update to separator used to separate simple products --- Component/Products.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Component/Products.php b/Component/Products.php index 4568513..6e38726 100644 --- a/Component/Products.php +++ b/Component/Products.php @@ -275,7 +275,7 @@ public function constructConfigurableVariations($data) $productsCount = count($products); $count = 0; foreach ($products as $sku) { - if ($count > $productsCount) { + if ($count > 0 && $count < $productsCount) { $variations .= '|'; } $productModel = $this->productFactory->create(); From 01cb5ab4283f2008409f9198a77d6abe8a2852eb Mon Sep 17 00:00:00 2001 From: Paul Partington Date: Thu, 14 Jan 2021 10:31:04 +0000 Subject: [PATCH 11/18] Fix the arguments to the product test. --- Test/Unit/Component/ProductsTest.php | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Test/Unit/Component/ProductsTest.php b/Test/Unit/Component/ProductsTest.php index 56e6699..d0079ee 100644 --- a/Test/Unit/Component/ProductsTest.php +++ b/Test/Unit/Component/ProductsTest.php @@ -8,6 +8,7 @@ use CtiDigital\Configurator\Component\Product\Image; use CtiDigital\Configurator\Component\Product\AttributeOption; use CtiDigital\Configurator\Api\LoggerInterface; +use CtiDigital\Configurator\Component\Product\ValidatorFactory; use Magento\Eav\Model\Entity\Attribute; class ProductsTest extends \PHPUnit\Framework\TestCase @@ -42,6 +43,11 @@ class ProductsTest extends \PHPUnit\Framework\TestCase */ private $log; + /** + * @var ValidatorFactory|\PHPUnit\Framework\MockObject\MockObject + */ + private $validatorFactory; + protected function setUp() { $this->importerFactory = $this->getMockBuilder(ImporterFactory::class) @@ -65,10 +71,15 @@ protected function setUp() ->disableOriginalConstructor() ->getMock(); + $this->validatorFactory = $this->getMockBuilder(ValidatorFactory::class) + ->disableOriginalConstructor() + ->getMock(); + $this->products = new Products( $this->importerFactory, $this->productFactory, $this->image, + $this->validatorFactory, $this->attributeOption, $this->log ); From 482afef93d79eb724e8df77821a662784028c9e9 Mon Sep 17 00:00:00 2001 From: Paul Partington Date: Thu, 14 Jan 2021 10:45:56 +0000 Subject: [PATCH 12/18] Code styling fixes. --- Component/Product/Validator.php | 34 ++++++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/Component/Product/Validator.php b/Component/Product/Validator.php index 059c4f0..65905c3 100644 --- a/Component/Product/Validator.php +++ b/Component/Product/Validator.php @@ -79,10 +79,20 @@ private function writeLog($rowData, $row, $attributeCode, $errorMessage, $type = $productIdentifierMessage = ($sku !== null) ? sprintf('SKU: %s', $sku) : sprintf('Row Number : %s', $row); switch ($type) { case self::IMPORT_DATA_ACTION_NULLIFY: - $message = sprintf('%s Error: %s Resolution: Unset the value for attribute code %s', $productIdentifierMessage, $errorMessage, $attributeCode); + $message = sprintf( + '%s Error: %s Resolution: Unset the value for attribute code %s', + $productIdentifierMessage, + $errorMessage, + $attributeCode + ); break; default: - $message = sprintf('%s Error: %s Resolution: Removed the row due to error with attribute code %s', $productIdentifierMessage, $errorMessage, $attributeCode); + $message = sprintf( + '%s Error: %s Resolution: Removed the row due to error with attribute code %s', + $productIdentifierMessage, + $errorMessage, + $attributeCode + ); $this->removedRows[$row] = $message; break; } @@ -146,13 +156,25 @@ public function omitItemsFromImport(array $importLines, array $failedRows) switch ($failedRow['action']) { case self::IMPORT_DATA_ACTION_NULLIFY: if (isset($importLines[$row][$attributeCode])) { - $this->writeLog($importLines[$row], $row, $attributeCode, $failedRow['message'], self::IMPORT_DATA_ACTION_NULLIFY); + $this->writeLog( + $importLines[$row], + $row, + $attributeCode, + $failedRow['message'], + self::IMPORT_DATA_ACTION_NULLIFY + ); $importLines[$row][$attributeCode] = null; } break; default: if (isset($importLines[$row])) { - $this->writeLog($importLines[$row], $row, $attributeCode, $failedRow['message'], self::IMPORT_DATA_ACTION_REMOVE); + $this->writeLog( + $importLines[$row], + $row, + $attributeCode, + $failedRow['message'], + self::IMPORT_DATA_ACTION_REMOVE + ); unset($importLines[$row]); } } @@ -194,7 +216,9 @@ private function formatRowRemoveData($error, $rows) }, $rows); $attributeCode = $this->getAttributeCodeFromError($error); - $action = (in_array($attributeCode, self::ATTRIBUTES_NULLIFY_ALLOW_LIST)) ? self::IMPORT_DATA_ACTION_NULLIFY : self::IMPORT_DATA_ACTION_REMOVE; + $action = (in_array($attributeCode, self::ATTRIBUTES_NULLIFY_ALLOW_LIST)) ? + self::IMPORT_DATA_ACTION_NULLIFY : + self::IMPORT_DATA_ACTION_REMOVE; return [ 'action' => $action, From b50b3fb5f330825114cb35ccf62f1d764c594bf3 Mon Sep 17 00:00:00 2001 From: Paul Partington Date: Thu, 14 Jan 2021 10:56:18 +0000 Subject: [PATCH 13/18] Remove 7.2 tests and update the Magento version. --- .travis.yml | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/.travis.yml b/.travis.yml index 298dc08..3a95236 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,6 @@ dist: trusty language: php php: - - 7.2 - 7.3 services: - mysql @@ -11,20 +10,10 @@ env: - TEST_SUITE=unit - TEST_SUITE=phpcs - TEST_SUITE=configurator - MAGE_VERSION=2.2.5 - TEST_SUITE=configurator - MAGE_VERSION=2.3.1 + MAGE_VERSION=2.3.6 - TEST_SUITE=configurator - MAGE_VERSION=2.3.2 - - TEST_SUITE=configurator - MAGE_VERSION=2.3.3 - -matrix: - allow_failures: - - php: 7.2 - - env: TEST_SUITE=configurator MAGE_VERSION=2.2.5 - - env: TEST_SUITE=configurator MAGE_VERSION=2.3.1 - - env: TEST_SUITE=configurator MAGE_VERSION=2.3.2 + MAGE_VERSION=2.4.1 before_install: - echo "{\"http-basic\":{\"repo.magento.com\":{\"username\":\"${MAGENTO_USERNAME}\",\"password\":\"${MAGENTO_PASSWORD}\"}}}" > auth.json From a48ec983f04de784f060fa2c58aa052b72a223fd Mon Sep 17 00:00:00 2001 From: Paul Partington Date: Thu, 14 Jan 2021 11:23:40 +0000 Subject: [PATCH 14/18] Attempt to remove Xdebug 3. --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 3a95236..df2aafd 100644 --- a/.travis.yml +++ b/.travis.yml @@ -16,6 +16,7 @@ env: MAGE_VERSION=2.4.1 before_install: + - phpenv config-rm xdebug.ini || true - echo "{\"http-basic\":{\"repo.magento.com\":{\"username\":\"${MAGENTO_USERNAME}\",\"password\":\"${MAGENTO_PASSWORD}\"}}}" > auth.json - sh -c "if [ '$TEST_SUITE' = 'phpcs' ]; then composer require magento/framework:^103.0.1; fi" - sh -c "if [ '$TEST_SUITE' = 'unit' ]; then composer require magento/framework:^103.0.1; fi" From a4a815a6d08277c56b40587a78ea0b04d0af0fab Mon Sep 17 00:00:00 2001 From: Paul Partington Date: Thu, 14 Jan 2021 11:29:06 +0000 Subject: [PATCH 15/18] Try to make sure Composer 1 is used. --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index df2aafd..80f6c8a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,7 +9,6 @@ env: matrix: - TEST_SUITE=unit - TEST_SUITE=phpcs - - TEST_SUITE=configurator - TEST_SUITE=configurator MAGE_VERSION=2.3.6 - TEST_SUITE=configurator @@ -17,6 +16,7 @@ env: before_install: - phpenv config-rm xdebug.ini || true + - composer self-update --1 - echo "{\"http-basic\":{\"repo.magento.com\":{\"username\":\"${MAGENTO_USERNAME}\",\"password\":\"${MAGENTO_PASSWORD}\"}}}" > auth.json - sh -c "if [ '$TEST_SUITE' = 'phpcs' ]; then composer require magento/framework:^103.0.1; fi" - sh -c "if [ '$TEST_SUITE' = 'unit' ]; then composer require magento/framework:^103.0.1; fi" From c76dee0a889d99ed77a87ae27b96f2b79f700806 Mon Sep 17 00:00:00 2001 From: Paul Partington Date: Thu, 14 Jan 2021 11:44:14 +0000 Subject: [PATCH 16/18] Code sniffer updates. --- Component/Product/Validator.php | 6 +++--- Component/Products.php | 7 ++++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/Component/Product/Validator.php b/Component/Product/Validator.php index 65905c3..bb423f0 100644 --- a/Component/Product/Validator.php +++ b/Component/Product/Validator.php @@ -76,12 +76,12 @@ public function getRemovedRows() private function writeLog($rowData, $row, $attributeCode, $errorMessage, $type = self::IMPORT_DATA_ACTION_REMOVE) { $sku = isset($rowData['sku']) ? $rowData['sku'] : null; - $productIdentifierMessage = ($sku !== null) ? sprintf('SKU: %s', $sku) : sprintf('Row Number : %s', $row); + $identifierMessage = ($sku !== null) ? sprintf('SKU: %s', $sku) : sprintf('Row Number : %s', $row); switch ($type) { case self::IMPORT_DATA_ACTION_NULLIFY: $message = sprintf( '%s Error: %s Resolution: Unset the value for attribute code %s', - $productIdentifierMessage, + $identifierMessage, $errorMessage, $attributeCode ); @@ -89,7 +89,7 @@ private function writeLog($rowData, $row, $attributeCode, $errorMessage, $type = default: $message = sprintf( '%s Error: %s Resolution: Removed the row due to error with attribute code %s', - $productIdentifierMessage, + $identifierMessage, $errorMessage, $attributeCode ); diff --git a/Component/Products.php b/Component/Products.php index 6e38726..717ef23 100644 --- a/Component/Products.php +++ b/Component/Products.php @@ -195,13 +195,13 @@ public function execute($data = null) * @var Validator $validatorModel */ $validatorModel = $this->validatorFactory->create(); - $validatedProductsArray = $validatorModel->getValidatedImport($validatorImport, $productsArray); + $validatedProducts = $validatorModel->getValidatedImport($validatorImport, $productsArray); $this->log->logInfo(sprintf('Removed %s products after validation.', count($validatorModel->getRemovedRows()))); - $this->log->logInfo(sprintf('Attempting to import %s rows', count($validatedProductsArray))); + $this->log->logInfo(sprintf('Attempting to import %s rows', count($validatedProducts))); try { $import = $this->importerFactory->create(); $import->setMultipleValueSeparator(self::SEPARATOR); - $import->processImport($validatedProductsArray); + $import->processImport($validatedProducts); } catch (\Exception $e) { $this->log->logError($e->getMessage()); } @@ -260,6 +260,7 @@ public function isConfigurable($data = []) /** * Create the configurable product string + * @SuppressWarnings(PHPMD.CyclomaticComplexity) * * @param $data * @return string From 3b91c9099f20f958c847aa74d77422a5a783f048 Mon Sep 17 00:00:00 2001 From: Paul Partington Date: Thu, 14 Jan 2021 12:17:51 +0000 Subject: [PATCH 17/18] Add Elasticsearch to Travis. --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 80f6c8a..1a3ce8b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,6 +4,7 @@ php: - 7.3 services: - mysql + - elasticsearch sudo: required env: matrix: From 5158bbe163edadf4e5131cc72c6a78b09f5c35ce Mon Sep 17 00:00:00 2001 From: Paul Partington Date: Thu, 14 Jan 2021 12:29:09 +0000 Subject: [PATCH 18/18] Remove 2.3.6 test. --- .travis.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 1a3ce8b..4f8f5ba 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,8 +10,6 @@ env: matrix: - TEST_SUITE=unit - TEST_SUITE=phpcs - - TEST_SUITE=configurator - MAGE_VERSION=2.3.6 - TEST_SUITE=configurator MAGE_VERSION=2.4.1