Skip to content
This repository was archived by the owner on Apr 29, 2019. It is now read-only.

import-export-improvements #82 : super attribute error message improvements #115

Merged
merged 8 commits into from
Jul 21, 2018
Merged

import-export-improvements #82 : super attribute error message improvements #115

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e99c99e
import-export-improvements #82 : configurable variations - not a supe…
Jun 28, 2018
65bce0a
import-export-improvements #82 : configurable variations - not a supe…
Jun 28, 2018
7f9f6db
import-export-improvements #82 : configurable variations - not a supe…
Jun 29, 2018
a76e3a3
import-export-improvements #82 : configurable variations - not a supe…
Jun 29, 2018
3a0599d
import-export-improvements #82 : configurable variations - not a supe…
Jul 6, 2018
6a5b15d
import-export-improvements #82 : configurable variations - not a supe…
Jul 6, 2018
b98f417
import-export-improvements #82 : configurable variations - not a supe…
Jul 7, 2018
8db836b
import-export-improvements #82 : configurable variations - not a supe…
Jul 7, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
import-export-improvements #82 : configurable variations - not a supe… 
…r attribute error message improvements - travis ci build code style fix
  • Loading branch information
Tadhg Bowe committed Jun 29, 2018
commit 7f9f6db9510508aae12260f517eba7e5ba198742
26 changes: 11 additions & 15 deletions app/code/Magento/ConfigurableImportExport/Model/Import/Product/Type/Configurable.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,13 @@ class Configurable extends \Magento\CatalogImportExport\Model\Import\Product\Typ

const ERROR_UNIDENTIFIABLE_VARIATION = 'unidentifiableVariation';

// @codingStandardsIgnoreStart
/**
* Validation failure message template definitions
*
* @var array
*
* Note: Many of these messages exceed maximum limit of 120 characters. Ignore from coding standards.
*/
protected $_messageTemplates = [
self::ERROR_ATTRIBUTE_CODE_DOES_NOT_EXIST => 'Column configurable_variations: Attribute with code "%s" does not exist or is missing from product attribute set',
Copy link
Contributor

@dmanners dmanners Jul 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have seen a few of these line issues before and people will format the array so the key and value are on two different lines. Would that make lines under 120 characters? Also does that make sense from your point of view?

Expand All @@ -56,6 +59,7 @@ class Configurable extends \Magento\CatalogImportExport\Model\Import\Product\Typ
self::ERROR_DUPLICATED_VARIATIONS => 'SKU %s contains duplicated variations',
self::ERROR_UNIDENTIFIABLE_VARIATION => 'Configurable variation "%s" is unidentifiable',
];
// @codingStandardsIgnoreEnd

/**
* Column names that holds values with particular meaning.
Expand Down Expand Up @@ -300,8 +304,7 @@ protected function _isParticularAttributesValid(array $rowData, $rowNum)
$superAttrCode = $rowData['_super_attribute_code'];
if (!$this->_isAttributeSuper($superAttrCode)) {
// Identify reason why attribute is not super:
if (!$this->_identifySuperAttributeError($superAttrCode, $rowNum))
{
if (!$this->_identifySuperAttributeError($superAttrCode, $rowNum)) {
Copy link
Contributor

@dmanners dmanners Jul 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall I like adding in this new check to give some more useful information to the user.

$this->_entityModel->addRowError(self::ERROR_ATTRIBUTE_CODE_IS_NOT_SUPER, $rowNum, $superAttrCode);
}
return false;
Expand Down Expand Up @@ -334,36 +337,29 @@ protected function _identifySuperAttributeError($superAttrCode, $rowNum)
// Does this attribute code exist? Does is have the correct settings?
$commonAttributes = self::$commonAttributesCache;
Copy link
Contributor

@dmanners dmanners Jul 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this assignment or could we just loop through the cache directly?

foreach ($commonAttributes as $attributeRow) {
Copy link
Contributor

@dmanners dmanners Jul 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there no way to access the attribute from the cache via the code rather than looping through all attributes. This could be a big performance hit if you have a large amount of attributes.


if ($attributeRow['code'] == $superAttrCode)
{
if ($attributeRow['code'] == $superAttrCode) {
$codeExists = true;

if ($attributeRow['is_global'] !== '1')
{
if ($attributeRow['is_global'] !== '1') {
$codeNotGlobal = true;
}
elseif ($attributeRow['type'] !== 'select')
{
elseif ($attributeRow['type'] !== 'select') {
$codeNotTypeSelect = true;
}

break;
}
}

if ($codeExists == false)
{
if ($codeExists == false) {
Copy link
Contributor

@dmanners dmanners Jul 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are setting these attributes let's do a === check here just to be sure we are dealing with booleans.

$this->_entityModel->addRowError(self::ERROR_ATTRIBUTE_CODE_DOES_NOT_EXIST, $rowNum, $superAttrCode);
$reasonFound = true;
}
elseif ($codeNotGlobal == true)
{
elseif ($codeNotGlobal == true) {
$this->_entityModel->addRowError(self::ERROR_ATTRIBUTE_CODE_NOT_GLOBAL_SCOPE, $rowNum, $superAttrCode);
$reasonFound = true;
}
elseif ($codeNotTypeSelect == true)
{
elseif ($codeNotTypeSelect == true) {
$this->_entityModel->addRowError(self::ERROR_ATTRIBUTE_CODE_NOT_TYPE_SELECT, $rowNum, $superAttrCode);
$reasonFound = true;
}
Expand Down