Skip to content

Commit 565edc9

Browse files
duhonduhon
authored
Merge pull request #2377 from magento-panda/PANDA-FIXES-2.2
### Bug * [MAGETWO-84942](https://jira.corp.magento.com/browse/MAGETWO-84942) [Magento cloud] Custom options should be cleared if custom options in import file is empty * [MAGETWO-89281](https://jira.corp.magento.com/browse/MAGETWO-89281) Files and folders symlinked in pub/media cannot be deleted from media gallery browser
2 parents b208fc4 + d64ec75 commit 565edc9

File tree

13 files changed

+380
-105
lines changed

13 files changed

+380
-105
lines changed

‎app/code/Magento/CatalogImportExport/Model/Import/Product/Option.php

Lines changed: 69 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -797,15 +797,15 @@ protected function _addRowsErrors($errorCode, array $errorNumbers)
797797
protected function _validateMainRow(array $rowData, $rowNumber)
798798
{
799799
if (!empty($rowData[self::COLUMN_STORE]) && !array_key_exists(
800-
$rowData[self::COLUMN_STORE],
801-
$this->_storeCodeToId
802-
)
800+
$rowData[self::COLUMN_STORE],
801+
$this->_storeCodeToId
802+
)
803803
) {
804804
$this->_productEntity->addRowError(self::ERROR_INVALID_STORE, $rowNumber);
805805
} elseif (!empty($rowData[self::COLUMN_TYPE]) && !array_key_exists(
806-
$rowData[self::COLUMN_TYPE],
807-
$this->_specificTypes
808-
)
806+
$rowData[self::COLUMN_TYPE],
807+
$this->_specificTypes
808+
)
809809
) {
810810
// type
811811
$this->_productEntity->addRowError(self::ERROR_INVALID_TYPE, $rowNumber);
@@ -910,9 +910,9 @@ protected function _saveNewOptionData(array $rowData, $rowNumber)
910910
protected function _validateSecondaryRow(array $rowData, $rowNumber)
911911
{
912912
if (!empty($rowData[self::COLUMN_STORE]) && !array_key_exists(
913-
$rowData[self::COLUMN_STORE],
914-
$this->_storeCodeToId
915-
)
913+
$rowData[self::COLUMN_STORE],
914+
$this->_storeCodeToId
915+
)
916916
) {
917917
$this->_productEntity->addRowError(self::ERROR_INVALID_STORE, $rowNumber);
918918
} elseif (!empty($rowData[self::COLUMN_ROW_PRICE]) && !is_numeric(rtrim($rowData[self::COLUMN_ROW_PRICE], '%'))
@@ -1211,6 +1211,7 @@ protected function _importData()
12111211
$typeTitles = [];
12121212
$parentCount = [];
12131213
$childCount = [];
1214+
$optionsToRemove = [];
12141215

12151216
foreach ($bunch as $rowNumber => $rowData) {
12161217
if (isset($optionId, $valueId) && empty($rowData[PRODUCT::COL_STORE_VIEW_CODE])) {
@@ -1220,6 +1221,12 @@ protected function _importData()
12201221
$optionId = $nextOptionId;
12211222
$valueId = $nextValueId;
12221223
$multiRowData = $this->_getMultiRowFormat($rowData);
1224+
if (!empty($rowData[self::COLUMN_SKU]) && isset($this->_productsSkuToId[$rowData[self::COLUMN_SKU]])) {
1225+
$this->_rowProductId = $this->_productsSkuToId[$rowData[self::COLUMN_SKU]];
1226+
if (array_key_exists('custom_options', $rowData) && trim($rowData['custom_options']) === "") {
1227+
$optionsToRemove[] = $this->_rowProductId;
1228+
}
1229+
}
12231230
foreach ($multiRowData as $optionData) {
12241231
$combinedData = array_merge($rowData, $optionData);
12251232

@@ -1253,32 +1260,23 @@ protected function _importData()
12531260
}
12541261
}
12551262

1256-
// Save prepared custom options data !!!
1263+
// Remove all existing options if import behaviour is APPEND
1264+
// in other case remove options for products with empty "custom_options" row only
12571265
if ($this->getBehavior() != \Magento\ImportExport\Model\Import::BEHAVIOR_APPEND) {
12581266
$this->_deleteEntities(array_keys($products));
1267+
} elseif (!empty($optionsToRemove)) {
1268+
// Remove options for products with empty "custom_options" row
1269+
$this->_deleteEntities($optionsToRemove);
12591270
}
12601271

1272+
// Save prepared custom options data
12611273
if ($this->_isReadyForSaving($options, $titles, $typeValues)) {
1262-
if ($this->getBehavior() == \Magento\ImportExport\Model\Import::BEHAVIOR_APPEND) {
1263-
$this->_compareOptionsWithExisting($options, $titles, $prices, $typeValues);
1264-
$this->restoreOriginalOptionTypeIds($typeValues, $typePrices, $typeTitles);
1265-
}
1266-
1267-
$this->_saveOptions(
1268-
$options
1269-
)->_saveTitles(
1270-
$titles
1271-
)->_savePrices(
1272-
$prices
1273-
)->_saveSpecificTypeValues(
1274-
$typeValues
1275-
)->_saveSpecificTypePrices(
1276-
$typePrices
1277-
)->_saveSpecificTypeTitles(
1278-
$typeTitles
1279-
)->_updateProducts(
1280-
$products
1281-
);
1274+
$types = [
1275+
'values' => $typeValues,
1276+
'prices' => $typePrices,
1277+
'titles' => $typeTitles
1278+
];
1279+
$this->savePreparedCustomOptions($products, $options, $titles, $prices, $types);
12821280
}
12831281
}
12841282

@@ -1523,12 +1521,9 @@ private function getExistingOptionTypeId($optionId, $storeId, $optionTypeTitle)
15231521
*/
15241522
protected function _parseRequiredData(array $rowData)
15251523
{
1526-
if ($rowData[self::COLUMN_SKU] != '' && isset($this->_productsSkuToId[$rowData[self::COLUMN_SKU]])) {
1527-
$this->_rowProductId = $this->_productsSkuToId[$rowData[self::COLUMN_SKU]];
1528-
} elseif (!isset($this->_rowProductId)) {
1524+
if (!isset($this->_rowProductId)) {
15291525
return false;
15301526
}
1531-
15321527
// Init store
15331528
if (!empty($rowData[self::COLUMN_STORE])) {
15341529
if (!isset($this->_storeCodeToId[$rowData[self::COLUMN_STORE]])) {
@@ -1982,4 +1977,44 @@ private function getProductIdentifierField()
19821977
}
19831978
return $this->productEntityIdentifierField;
19841979
}
1980+
1981+
/**
1982+
* Save prepared custom options
1983+
*
1984+
* @param array $products
1985+
* @param array $options
1986+
* @param array $titles
1987+
* @param array $prices
1988+
* @param array $types
1989+
*
1990+
* @return void
1991+
*/
1992+
private function savePreparedCustomOptions(
1993+
array $products,
1994+
array $options,
1995+
array $titles,
1996+
array $prices,
1997+
array $types
1998+
) {
1999+
if ($this->getBehavior() == \Magento\ImportExport\Model\Import::BEHAVIOR_APPEND) {
2000+
$this->_compareOptionsWithExisting($options, $titles, $prices, $types['values']);
2001+
$this->restoreOriginalOptionTypeIds($types['values'], $types['prices'], $types['titles']);
2002+
}
2003+
2004+
$this->_saveOptions(
2005+
$options
2006+
)->_saveTitles(
2007+
$titles
2008+
)->_savePrices(
2009+
$prices
2010+
)->_saveSpecificTypeValues(
2011+
$types['values']
2012+
)->_saveSpecificTypePrices(
2013+
$types['prices']
2014+
)->_saveSpecificTypeTitles(
2015+
$types['titles']
2016+
)->_updateProducts(
2017+
$products
2018+
);
2019+
}
19852020
}

‎app/code/Magento/Cms/Model/Wysiwyg/Images/Storage.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -735,7 +735,7 @@ protected function _validatePath($path)
735735
*/
736736
protected function _sanitizePath($path)
737737
{
738-
return rtrim(preg_replace('~[/\\\]+~', '/', $this->_directory->getDriver()->getRealPath($path)), '/');
738+
return rtrim(preg_replace('~[/\\\]+~', '/', $this->_directory->getDriver()->getRealPathSafety($path)), '/');
739739
}
740740

741741
/**

‎app/code/Magento/Cms/Test/Unit/Model/Wysiwyg/Images/StorageTest.php

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -114,16 +114,9 @@ class StorageTest extends \PHPUnit\Framework\TestCase
114114
protected function setUp()
115115
{
116116
$this->filesystemMock = $this->createMock(\Magento\Framework\Filesystem::class);
117-
$this->driverMock = $this->getMockForAbstractClass(
118-
\Magento\Framework\Filesystem\DriverInterface::class,
119-
[],
120-
'',
121-
false,
122-
false,
123-
true,
124-
['getRealPath']
125-
);
126-
$this->driverMock->expects($this->any())->method('getRealPath')->will($this->returnArgument(0));
117+
$this->driverMock = $this->getMockBuilder(\Magento\Framework\Filesystem\DriverInterface::class)
118+
->setMethods(['getRealPathSafety'])
119+
->getMockForAbstractClass();
127120

128121
$this->directoryMock = $this->createPartialMock(
129122
\Magento\Framework\Filesystem\Directory\Write::class,
@@ -243,6 +236,7 @@ public function testDeleteDirectoryOverRoot()
243236
\Magento\Framework\Exception\LocalizedException::class,
244237
sprintf('Directory %s is not under storage root path.', self::INVALID_DIRECTORY_OVER_ROOT)
245238
);
239+
$this->driverMock->expects($this->atLeastOnce())->method('getRealPathSafety')->will($this->returnArgument(0));
246240
$this->imagesStorage->deleteDirectory(self::INVALID_DIRECTORY_OVER_ROOT);
247241
}
248242

@@ -255,6 +249,7 @@ public function testDeleteRootDirectory()
255249
\Magento\Framework\Exception\LocalizedException::class,
256250
sprintf('We can\'t delete root directory %s right now.', self::STORAGE_ROOT_DIR)
257251
);
252+
$this->driverMock->expects($this->atLeastOnce())->method('getRealPathSafety')->will($this->returnArgument(0));
258253
$this->imagesStorage->deleteDirectory(self::STORAGE_ROOT_DIR);
259254
}
260255

‎dev/tests/integration/testsuite/Magento/CatalogImportExport/Model/Import/ProductTest.php

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -279,9 +279,10 @@ public function testStockState()
279279
* @dataProvider getBehaviorDataProvider
280280
* @param string $importFile
281281
* @param string $sku
282+
* @param int $expectedOptionsQty
282283
* @magentoAppIsolation enabled
283284
*/
284-
public function testSaveCustomOptions($importFile, $sku)
285+
public function testSaveCustomOptions($importFile, $sku, $expectedOptionsQty)
285286
{
286287
$pathToFile = __DIR__ . '/_files/' . $importFile;
287288
$importModel = $this->createImportModel($pathToFile);
@@ -314,6 +315,7 @@ public function testSaveCustomOptions($importFile, $sku)
314315
// assert of options data
315316
$this->assertCount(count($expectedData['data']), $actualData['data']);
316317
$this->assertCount(count($expectedData['values']), $actualData['values']);
318+
$this->assertCount($expectedOptionsQty, $actualData['options']);
317319
foreach ($expectedData['options'] as $expectedId => $expectedOption) {
318320
$elementExist = false;
319321
// find value in actual options and values
@@ -418,12 +420,19 @@ public function getBehaviorDataProvider()
418420
{
419421
return [
420422
'Append behavior with existing product' => [
421-
'$importFile' => 'product_with_custom_options.csv',
422-
'$sku' => 'simple',
423+
'importFile' => 'product_with_custom_options.csv',
424+
'sku' => 'simple',
425+
'expectedOptionsQty' => 6
426+
],
427+
'Append behavior with existing product and without options in import file' => [
428+
'importFile' => 'product_without_custom_options.csv',
429+
'sku' => 'simple',
430+
'expectedOptionsQty' => 0
423431
],
424432
'Append behavior with new product' => [
425-
'$importFile' => 'product_with_custom_options_new.csv',
426-
'$sku' => 'simple_new',
433+
'importFile' => 'product_with_custom_options_new.csv',
434+
'sku' => 'simple_new',
435+
'expectedOptionsQty' => 4
427436
]
428437
];
429438
}
@@ -574,6 +583,7 @@ protected function getExpectedOptionsData($pathToFile, $storeCode = '')
574583
break;
575584
}
576585
}
586+
if (!empty($productData['data'][$storeRowId]['custom_options'])) {
577587
foreach (explode('|', $productData['data'][$storeRowId]['custom_options']) as $optionData) {
578588
$option = array_values(
579589
array_map(
@@ -595,7 +605,7 @@ function ($input) {
595605
$expectedData[$expectedOptionId] = [];
596606
foreach ($this->_assertOptions as $assertKey => $assertFieldName) {
597607
if (array_key_exists($assertFieldName, $option)
598-
&& !(($assertFieldName == 'price' || $assertFieldName == 'sku')
608+
&& !(($assertFieldName == 'price' || $assertFieldName == 'sku')
599609
&& in_array($option['type'], $this->specificTypes))
600610
) {
601611
$expectedData[$expectedOptionId][$assertKey] = $option[$assertFieldName];
@@ -613,6 +623,7 @@ function ($input) {
613623
$expectedValues[$expectedOptionId][] = $optionValue;
614624
}
615625
}
626+
}
616627
return [
617628
'id' => $expectedOptionId,
618629
'options' => $expectedOptions,

‎dev/tests/integration/testsuite/Magento/CatalogImportExport/Model/Import/_files/product_without_custom_options.csv

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
sku,website_code,store_view_code,attribute_set_code,product_type,name,description,short_description,weight,product_online,visibility,product_websites,categories,price,special_price,special_price_from_date,special_price_to_date,tax_class_name,url_key,meta_title,meta_keywords,meta_description,base_image,base_image_label,small_image,small_image_label,thumbnail_image,thumbnail_image_label,additional_images,additional_image_labels,configurable_variation_labels,configurable_variations,bundle_price_type,bundle_sku_type,bundle_weight_type,bundle_values,downloadble_samples,downloadble_links,associated_skus,related_skus,crosssell_skus,upsell_skus,custom_options,additional_attributes,manage_stock,is_in_stock,qty,out_of_stock_qty,is_qty_decimal,allow_backorders,min_cart_qty,max_cart_qty,notify_on_stock_below,qty_increments,enable_qty_increments,is_decimal_divided,new_from_date,new_to_date,gift_message_available,created_at,updated_at,custom_design,custom_design_from,custom_design_to,custom_layout_update,page_layout,product_options_container,msrp_price,msrp_display_actual_price_type,map_enabled
2+
simple,base,,Default,simple,New Product,,,9,1,"Catalog, Search",base,,10,,,,Taxable Goods,new-product,,,,,,,,,,,,,,,,,,,,,,,,,,1,1,999,0,0,0,1,10000,1,1,0,0,,,,,,,,,,,Block after Info Column,,,

‎dev/tests/integration/testsuite/Magento/Cms/Controller/Adminhtml/Wysiwyg/Images/DeleteFilesTest.php

Lines changed: 46 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -31,30 +31,29 @@ class DeleteFilesTest extends \PHPUnit\Framework\TestCase
3131
/**
3232
* @var string
3333
*/
34-
private $fullDirectoryPath;
34+
private $fileName = 'magento_small_image.jpg';
3535

3636
/**
37-
* @var string
37+
* @var \Magento\Framework\Filesystem
3838
*/
39-
private $fileName = 'magento_small_image.jpg';
39+
private $filesystem;
40+
41+
/**
42+
* @var \Magento\Framework\ObjectManagerInterface
43+
*/
44+
private $objectManager;
4045

4146
/**
4247
* @inheritdoc
4348
*/
4449
protected function setUp()
4550
{
46-
$objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager();
47-
$directoryName = 'directory1';
48-
$filesystem = $objectManager->get(\Magento\Framework\Filesystem::class);
51+
$this->objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager();
52+
$this->filesystem = $this->objectManager->get(\Magento\Framework\Filesystem::class);
4953
/** @var \Magento\Cms\Helper\Wysiwyg\Images $imagesHelper */
50-
$this->imagesHelper = $objectManager->get(\Magento\Cms\Helper\Wysiwyg\Images::class);
51-
$this->mediaDirectory = $filesystem->getDirectoryWrite(DirectoryList::MEDIA);
52-
$this->fullDirectoryPath = $this->imagesHelper->getStorageRoot() . '/' . $directoryName;
53-
$this->mediaDirectory->create($this->mediaDirectory->getRelativePath($this->fullDirectoryPath));
54-
$filePath = $this->fullDirectoryPath . DIRECTORY_SEPARATOR . $this->fileName;
55-
$fixtureDir = realpath(__DIR__ . '/../../../../../Catalog/_files');
56-
copy($fixtureDir . '/' . $this->fileName, $filePath);
57-
$this->model = $objectManager->get(\Magento\Cms\Controller\Adminhtml\Wysiwyg\Images\DeleteFiles::class);
54+
$this->imagesHelper = $this->objectManager->get(\Magento\Cms\Helper\Wysiwyg\Images::class);
55+
$this->mediaDirectory = $this->filesystem->getDirectoryWrite(DirectoryList::MEDIA);
56+
$this->model = $this->objectManager->get(\Magento\Cms\Controller\Adminhtml\Wysiwyg\Images\DeleteFiles::class);
5857
}
5958

6059
/**
@@ -65,17 +64,48 @@ protected function setUp()
6564
*/
6665
public function testExecute()
6766
{
67+
$directoryName = 'directory1';
68+
$fullDirectoryPath = $this->imagesHelper->getStorageRoot() . '/' . $directoryName;
69+
$this->mediaDirectory->create($this->mediaDirectory->getRelativePath($fullDirectoryPath));
70+
$filePath = $fullDirectoryPath . DIRECTORY_SEPARATOR . $this->fileName;
71+
$fixtureDir = realpath(__DIR__ . '/../../../../../Catalog/_files');
72+
copy($fixtureDir . '/' . $this->fileName, $filePath);
73+
6874
$this->model->getRequest()->setMethod('POST')
6975
->setPostValue('files', [$this->imagesHelper->idEncode($this->fileName)]);
70-
$this->model->getStorage()->getSession()->setCurrentPath($this->fullDirectoryPath);
76+
$this->model->getStorage()->getSession()->setCurrentPath($fullDirectoryPath);
7177
$this->model->execute();
7278
$this->assertFalse(
7379
$this->mediaDirectory->isExist(
74-
$this->mediaDirectory->getRelativePath($this->fullDirectoryPath . '/' . $this->fileName)
80+
$this->mediaDirectory->getRelativePath($fullDirectoryPath . '/' . $this->fileName)
7581
)
7682
);
7783
}
7884

85+
/**
86+
* Execute method with correct directory path and file name to check that files under linked media directory
87+
* can be removed.
88+
*
89+
* @return void
90+
* @magentoDataFixture Magento/Cms/_files/linked_media.php
91+
*/
92+
public function testExecuteWithLinkedMedia()
93+
{
94+
$directoryName = 'linked_media';
95+
$fullDirectoryPath = $this->filesystem->getDirectoryRead(DirectoryList::PUB)
96+
->getAbsolutePath() . DIRECTORY_SEPARATOR . $directoryName;
97+
$filePath = $fullDirectoryPath . DIRECTORY_SEPARATOR . $this->fileName;
98+
$fixtureDir = realpath(__DIR__ . '/../../../../../Catalog/_files');
99+
copy($fixtureDir . '/' . $this->fileName, $filePath);
100+
101+
$wysiwygDir = $this->mediaDirectory->getAbsolutePath() . '/wysiwyg';
102+
$this->model->getRequest()->setMethod('POST')
103+
->setPostValue('files', [$this->imagesHelper->idEncode($this->fileName)]);
104+
$this->model->getStorage()->getSession()->setCurrentPath($wysiwygDir);
105+
$this->model->execute();
106+
$this->assertFalse(is_file($fullDirectoryPath . DIRECTORY_SEPARATOR . $this->fileName));
107+
}
108+
79109
/**
80110
* @inheritdoc
81111
*/

0 commit comments

Comments
 (0)