Skip to content

Limit the php explode to 2 to prevent extra '=' sign content in the a… #25194

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jan 17, 2020
Merged

Limit the php explode to 2 to prevent extra '=' sign content in the a… #25194

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
cded4b1
Limit the php explode to 2 to prevent extra '=' sign content in the a…
Oct 21, 2019
904b234
Limit the php explode to 2 to prevent extra '=' sign content in the a…
Oct 21, 2019
c983d41
Merge branch '2.4-develop' into bug-fix/issue-number_25188
swnsma Dec 16, 2019
c721ae4
cover changes with unit test
Nazar65 Jan 8, 2020
e9226fe
Merge branch '2.4-develop' of https://github.com/magento/magento2 int…
Nazar65 Jan 8, 2020
2dc4f1e
static test fix
Nazar65 Jan 8, 2020
2dd6b14
fix phpStan
Nazar65 Jan 8, 2020
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
2 changes: 1 addition & 1 deletion 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 @@ -573,7 +573,7 @@ protected function _parseVariations($rowData)

$fieldAndValuePairs = [];
foreach ($fieldAndValuePairsText as $nameAndValue) {
$nameAndValue = explode(ImportProduct::PAIR_NAME_VALUE_SEPARATOR, $nameAndValue);
$nameAndValue = explode(ImportProduct::PAIR_NAME_VALUE_SEPARATOR, $nameAndValue, 2);
if (!empty($nameAndValue)) {
$value = isset($nameAndValue[1]) ? trim($nameAndValue[1]) : '';
// Ignoring field names' case.
Expand Down
169 changes: 103 additions & 66 deletions ...Magento/ConfigurableImportExport/Test/Unit/Model/Import/Product/Type/ConfigurableTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
use Magento\ConfigurableImportExport;

/**
* Class ConfigurableTest
* @package Magento\ConfigurableImportExport\Test\Unit\Model\Import\Product\Type
* Configurable import export tests
*
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
*/
class ConfigurableTest extends \Magento\ImportExport\Test\Unit\Model\Import\AbstractImportTestCase
Expand Down Expand Up @@ -78,6 +78,8 @@ class ConfigurableTest extends \Magento\ImportExport\Test\Unit\Model\Import\Abst
protected $productEntityLinkField = 'entity_id';

/**
* @inheritdoc
*
* @SuppressWarnings(PHPMD.ExcessiveMethodLength)
*/
protected function setUp()
Expand Down Expand Up @@ -270,10 +272,12 @@ protected function setUp()
}

/**
* Bunches data provider
*
* @return array
* @SuppressWarnings(PHPMD.ExcessiveMethodLength)
*/
protected function _getBunch()
protected function _getBunch(): array
{
return [[
'sku' => 'configurableskuI22',
Expand Down Expand Up @@ -393,9 +397,11 @@ protected function _getBunch()
}

/**
* Super attributes data provider
*
* @return array
*/
protected function _getSuperAttributes()
protected function _getSuperAttributes(): array
{
return [
'testattr2' => [
Expand All @@ -404,7 +410,6 @@ protected function _getSuperAttributes()
'attribute_code' => 'testattr2',
'is_global' => '1',
'is_visible' => '1',
'is_static' => '0',
'is_required' => '0',
'is_unique' => '0',
'frontend_label' => 'testattr2',
Expand All @@ -426,7 +431,6 @@ protected function _getSuperAttributes()
'attribute_code' => 'testattr3',
'is_global' => '1',
'is_visible' => '1',
'is_static' => '0',
'is_required' => '0',
'is_unique' => '0',
'frontend_label' => 'testattr3',
Expand All @@ -445,6 +449,8 @@ protected function _getSuperAttributes()
}

/**
* Verify save mtethod
*
* @SuppressWarnings(PHPMD.ExcessiveMethodLength)
*/
public function testSaveData()
Expand Down Expand Up @@ -527,6 +533,8 @@ public function testSaveData()
}

/**
* Callback for is row allowed to import
*
* @param $rowData
* @param $rowNum
* @return bool
Expand All @@ -540,83 +548,110 @@ public function isRowAllowedToImport($rowData, $rowNum)
return true;
}

public function testIsRowValid()
/**
* Verify is row valid method
*
* @dataProvider getProductDataIsValidRow
* @param array $productData
* @return void
*/
public function testIsRowValid(array $productData): void
{
$bunch = $this->_getBunch();
$badProduct = [
'sku' => 'configurableskuI22BadPrice',
'store_view_code' => null,
'attribute_set_code' => 'Default',
'product_type' => 'configurable',
'name' => 'Configurable Product 21 BadPrice',
'product_websites' => 'website_1',
'configurable_variation_labels' => 'testattr2=Select Color, testattr3=Select Size',
'configurable_variations' => 'sku=testconf2-attr2val1-testattr3v1,'
. 'testattr2=attr2val1_DOESNT_EXIST,'
. 'testattr3=testattr3v1,'
. 'display=1|sku=testconf2-attr2val1-testattr3v2,'
. 'testattr2=attr2val1,'
. 'testattr3=testattr3v2,'
. 'display=0',
'_store' => null,
'_attribute_set' => 'Default',
'_type' => 'configurable',
'_product_websites' => 'website_1',
];
// Checking that variations' field names are case-insensitive with this
// product.
$caseInsensitiveSKU = 'configurableskuI22CaseInsensitive';
$caseInsensitiveProduct = [
'sku' => $caseInsensitiveSKU,
'store_view_code' => null,
'attribute_set_code' => 'Default',
'product_type' => 'configurable',
'name' => 'Configurable Product 21',
'product_websites' => 'website_1',
'configurable_variation_labels' => 'testattr2=Select Color, testattr3=Select Size',
'configurable_variations' => 'SKU=testconf2-attr2val1-testattr3v1,'
. 'testattr2=attr2val1,'
. 'testattr3=testattr3v1,'
. 'display=1|sku=testconf2-attr2val1-testattr3v2,'
. 'testattr2=attr2val1,'
. 'testattr3=testattr3v2,'
. 'display=0',
'_store' => null,
'_attribute_set' => 'Default',
'_type' => 'configurable',
'_product_websites' => 'website_1',
];
$bunch[] = $badProduct;
$bunch[] = $caseInsensitiveProduct;
$productData['caseInsencitiveProduct']['sku'] = $caseInsensitiveSKU;
$bunch[] = $productData['bad_product'];
$bunch[] = $productData['caseInsencitiveProduct'];
// Set _attributes to avoid error in Magento\CatalogImportExport\Model\Import\Product\Type\AbstractType.
$this->setPropertyValue($this->configurable, '_attributes', [
$badProduct[\Magento\CatalogImportExport\Model\Import\Product::COL_ATTR_SET] => [],
$productData['bad_product'][\Magento\CatalogImportExport\Model\Import\Product::COL_ATTR_SET] => [],
]);
// Avoiding errors about attributes not being super
$this->setPropertyValue(
$this->configurable,
'_superAttributes',
[
'testattr2' => ['options' => ['attr2val1' => 1]],
'testattr3' => [
'options' => [
'testattr3v2' => 1,
'testattr3v1' => 1,
],
],
]
);
$this->setPropertyValue($this->configurable, '_superAttributes', $productData['super_attributes']);

foreach ($bunch as $rowData) {
$result = $this->configurable->isRowValid($rowData, 0, !isset($this->_oldSku[$rowData['sku']]));
$result = $this->configurable->isRowValid($rowData, 0, false);
$this->assertNotNull($result);
if ($rowData['sku'] === $caseInsensitiveSKU) {
$this->assertTrue($result);
}
}
}

public function testRowValidationForNumericalSkus()
/**
*
* Data provider for isValidRows test.
*
* @return array
*/
public function getProductDataIsValidRow(): array
{
return [
[
[
'bad_product' => [
'sku' => 'configurableskuI22BadPrice',
'store_view_code' => null,
'attribute_set_code' => 'Default',
'product_type' => 'configurable',
'name' => 'Configurable Product 21 BadPrice',
'product_websites' => 'website_1',
'configurable_variation_labels' => 'testattr2=Select Color, testattr3=Select Size',
'configurable_variations' => 'sku=testconf2-attr2val1-testattr3v1,'
. 'testattr2=attr2val1_DOESNT_EXIST,'
. 'testattr3=testattr3v1,'
. 'display=1|sku=testconf2-attr2val1-testattr3v2,'
. 'testattr2=attr2val1,'
. 'testattr3=testattr3v2,'
. 'display=0',
'_store' => null,
'_attribute_set' => 'Default',
'_type' => 'configurable',
'_product_websites' => 'website_1',
],
'caseInsencitiveProduct' => [
'sku' => '',
'store_view_code' => null,
'attribute_set_code' => 'Default',
'product_type' => 'configurable',
'name' => 'Configurable Product 21',
'product_websites' => 'website_1',
'configurable_variation_labels' => 'testattr2=Select Color, testattr3=Select Size',
'configurable_variations' => 'SKU=testconf2-attr2val1-testattr3v1,'
. 'testattr2=attr2val1,'
. 'testattr3=testattr3v1=sx=sl,'
. 'display=1|sku=testconf2-attr2val1-testattr3v2,'
. 'testattr2=attr2val1,'
. 'testattr3=testattr3v2,'
. 'display=0',
'_store' => null,
'_attribute_set' => 'Default',
'_type' => 'configurable',
'_product_websites' => 'website_1',
],
'super_attributes' =>
[
'testattr2' => ['options' => ['attr2val1' => 1]],
'testattr3' => [
'options' => [
'testattr3v2' => 1,
'testattr3v1=sx=sl' => 1,
],
],
]
]
]
];
}

/**
* Verify row validation with numeric skus
*
* @return void
*/
public function testRowValidationForNumericalSkus(): void
{
// Set _attributes to avoid error in Magento\CatalogImportExport\Model\Import\Product\Type\AbstractType.
$this->setPropertyValue($this->configurable, '_attributes', [
Expand Down Expand Up @@ -649,9 +684,11 @@ public function testRowValidationForNumericalSkus()
}

/**
* Row validation Data Provider
*
* @return array
*/
public function rowValidationDataProvider()
public function rowValidationDataProvider(): array
{
return [
'duplicateProduct' => [
Expand Down