-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Various fixes in Magento 2 core to support MSI #13434
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.
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
Changes from 9 commits
992bb67
420afc8
13efce8
9573d88
0067244
ad91424
1b8cc6a
36098f4
f016278
bffe557
d4e8e3b
6d5e882
c084214
3a2533b
6a3835f
0a49140
aba9b73
7fc59e7
661541a
bb6bac3
6f22a17
e4eab13
28b07ed
a509c71
be29aa7
aa365e0
af7737c
e949bdb
0dceea8
d62db82
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,103 @@ | ||
<?php | ||
/** | ||
* Copyright © Magento, Inc. All rights reserved. | ||
* See COPYING.txt for license details. | ||
*/ | ||
declare(strict_types=1); | ||
|
||
namespace Magento\Backend\Ui\Component\Control; | ||
|
||
use Magento\Framework\App\RequestInterface; | ||
use Magento\Framework\Escaper; | ||
use Magento\Framework\UrlInterface; | ||
use Magento\Framework\View\Element\UiComponent\Control\ButtonProviderInterface; | ||
|
||
/** | ||
* Represents delete button with pre-configured options | ||
* Provide an ability to show confirmation message on click on the "Delete" button | ||
* | ||
* @api | ||
*/ | ||
class DeleteButton implements ButtonProviderInterface | ||
{ | ||
/** | ||
* @var RequestInterface | ||
*/ | ||
private $request; | ||
|
||
/** | ||
* @var UrlInterface | ||
*/ | ||
private $urlBuilder; | ||
|
||
/** | ||
* @var Escaper | ||
*/ | ||
private $escaper; | ||
|
||
/** | ||
* @var string | ||
*/ | ||
private $confirmationMessage; | ||
|
||
/** | ||
* @var string | ||
*/ | ||
private $idFieldName; | ||
|
||
/** | ||
* @var string | ||
*/ | ||
private $deleteRoutePath; | ||
|
||
/** | ||
* @var int | ||
*/ | ||
private $sortOrder; | ||
|
||
/** | ||
* @param RequestInterface $request | ||
* @param UrlInterface $urlBuilder | ||
* @param string $confirmationMessage | ||
* @param string $idFieldName | ||
* @param string $deleteRoutePath | ||
* @param int $sortOrder | ||
*/ | ||
public function __construct( | ||
RequestInterface $request, | ||
UrlInterface $urlBuilder, | ||
Escaper $escaper, | ||
string $confirmationMessage, | ||
string $idFieldName, | ||
string $deleteRoutePath, | ||
int $sortOrder | ||
) { | ||
$this->request = $request; | ||
$this->urlBuilder = $urlBuilder; | ||
$this->escaper = $escaper; | ||
$this->confirmationMessage = $confirmationMessage; | ||
$this->idFieldName = $idFieldName; | ||
$this->deleteRoutePath = $deleteRoutePath; | ||
$this->sortOrder = $sortOrder; | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function getButtonData() | ||
{ | ||
$data = []; | ||
$fieldId = $this->request->getParam($this->idFieldName); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. First of all, we need to escape There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
if (null !== $fieldId) { | ||
$url = $this->urlBuilder->getUrl($this->deleteRoutePath); | ||
$escapedMessage = $this->escaper->escapeJs($this->escaper->escapeHtml($this->confirmationMessage)); | ||
$data = [ | ||
'label' => __('Delete'), | ||
'class' => 'delete', | ||
'on_click' => "deleteConfirm('{$escapedMessage}', '{$url}', {data:{{$this->idFieldName}:{$fieldId}}})", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Potential XSS via There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
'sort_order' => $this->sortOrder, | ||
]; | ||
} | ||
return $data; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,7 @@ | |
use Magento\CatalogImportExport\Model\Import\Product\MediaGalleryProcessor; | ||
use Magento\CatalogImportExport\Model\Import\Product\ImageTypeProcessor; | ||
use Magento\CatalogImportExport\Model\Import\Product\RowValidatorInterface as ValidatorInterface; | ||
use Magento\CatalogImportExport\Model\StockItemImporterInterface; | ||
use Magento\Framework\App\Filesystem\DirectoryList; | ||
use Magento\Framework\App\ObjectManager; | ||
use Magento\Framework\Exception\LocalizedException; | ||
|
@@ -703,6 +704,13 @@ class Product extends \Magento\ImportExport\Model\Import\Entity\AbstractEntity | |
*/ | ||
private $catalogConfig; | ||
|
||
/** | ||
* Stock Item Importer | ||
* | ||
* @var StockItemImporterInterface $stockItemImporter | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Currently Magento does not use any standard which requires adding variable name in this kind of docblock There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
*/ | ||
private $stockItemImporter; | ||
|
||
/** | ||
* @var ImageTypeProcessor | ||
*/ | ||
|
@@ -754,6 +762,7 @@ class Product extends \Magento\ImportExport\Model\Import\Entity\AbstractEntity | |
* @param array $data | ||
* @param array $dateAttrCodes | ||
* @param CatalogConfig $catalogConfig | ||
* @param StockItemImporterInterface $stockItemImporter | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Order of parameters in the docblock is different from the method signature There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
* @param ImageTypeProcessor $imageTypeProcessor | ||
* @param MediaGalleryProcessor $mediaProcessor | ||
* @throws \Magento\Framework\Exception\LocalizedException | ||
|
@@ -801,7 +810,8 @@ public function __construct( | |
array $dateAttrCodes = [], | ||
CatalogConfig $catalogConfig = null, | ||
ImageTypeProcessor $imageTypeProcessor = null, | ||
MediaGalleryProcessor $mediaProcessor = null | ||
MediaGalleryProcessor $mediaProcessor = null, | ||
StockItemImporterInterface $stockItemImporter = null | ||
) { | ||
$this->_eventManager = $eventManager; | ||
$this->stockRegistry = $stockRegistry; | ||
|
@@ -835,7 +845,8 @@ public function __construct( | |
$this->catalogConfig = $catalogConfig ?: ObjectManager::getInstance()->get(CatalogConfig::class); | ||
$this->imageTypeProcessor = $imageTypeProcessor ?: ObjectManager::getInstance()->get(ImageTypeProcessor::class); | ||
$this->mediaProcessor = $mediaProcessor ?: ObjectManager::getInstance()->get(MediaGalleryProcessor::class); | ||
|
||
$this->stockItemImporter = $stockItemImporter ?: ObjectManager::getInstance() | ||
->get(StockItemImporterInterface::class); | ||
parent::__construct( | ||
$jsonHelper, | ||
$importExportData, | ||
|
@@ -851,7 +862,6 @@ public function __construct( | |
) ? $data['option_entity'] : $optionFactory->create( | ||
['data' => ['product_entity' => $this]] | ||
); | ||
|
||
$this->_initAttributeSets() | ||
->_initTypeModels() | ||
->_initSkus() | ||
|
@@ -2128,9 +2138,6 @@ protected function _saveProductWebsites(array $websiteData) | |
*/ | ||
protected function _saveStockItem() | ||
{ | ||
/** @var $stockResource \Magento\CatalogInventory\Model\ResourceModel\Stock\Item */ | ||
$stockResource = $this->_stockResItemFac->create(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like it was the only usage of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
$entityTable = $stockResource->getMainTable(); | ||
while ($bunch = $this->_dataSourceModel->getNextBunch()) { | ||
$stockData = []; | ||
$productIdsToReindex = []; | ||
|
@@ -2158,6 +2165,7 @@ protected function _saveStockItem() | |
array_intersect_key($rowData, $this->defaultStockData), | ||
$row | ||
); | ||
$row['sku'] = $sku; | ||
|
||
if ($this->stockConfiguration->isQty( | ||
$this->skuProcessor->getNewSku($sku)['type_id'] | ||
|
@@ -2185,7 +2193,7 @@ protected function _saveStockItem() | |
|
||
// Insert rows | ||
if (!empty($stockData)) { | ||
$this->_connection->insertOnDuplicate($entityTable, array_values($stockData)); | ||
$this->stockItemImporter->import($stockData); | ||
} | ||
|
||
$this->reindexProducts($productIdsToReindex); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
<?php | ||
/** | ||
* Copyright © Magento, Inc. All rights reserved. | ||
* See COPYING.txt for license details. | ||
*/ | ||
declare(strict_types=1); | ||
|
||
namespace Magento\CatalogImportExport\Model; | ||
|
||
use Magento\CatalogInventory\Model\ResourceModel\Stock\Item; | ||
use Magento\CatalogInventory\Model\ResourceModel\Stock\ItemFactory; | ||
use Magento\Framework\Exception\CouldNotSaveException; | ||
use Psr\Log\LoggerInterface; | ||
|
||
class StockItemImporter implements StockItemImporterInterface | ||
{ | ||
/** | ||
* Stock Item Resource Factory | ||
* | ||
* @var ItemFactory $stockResourceItemFactory | ||
*/ | ||
private $stockResourceItemFactory; | ||
|
||
/** | ||
* @var LoggerInterface | ||
*/ | ||
private $logger; | ||
|
||
/** | ||
* StockItemImporter constructor | ||
* | ||
* @param ItemFactory $stockResourceItemFactory | ||
* @param LoggerInterface $logger | ||
*/ | ||
public function __construct( | ||
ItemFactory $stockResourceItemFactory, | ||
LoggerInterface $logger | ||
) { | ||
$this->stockResourceItemFactory = $stockResourceItemFactory; | ||
$this->logger = $logger; | ||
} | ||
|
||
/** | ||
* Handle Import of Stock Item Data | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
* | ||
* @param array $stockData | ||
* @return void | ||
* @throws CouldNotSaveException | ||
*/ | ||
public function import(array $stockData) | ||
{ | ||
/** @var $stockItemResource Item */ | ||
$stockItemResource = $this->stockResourceItemFactory->create(); | ||
$entityTable = $stockItemResource->getMainTable(); | ||
try { | ||
$stockImportData = array_map( | ||
function ($stockItemData) { | ||
unset($stockItemData['sku']); | ||
return $stockItemData; | ||
}, | ||
array_values($stockData) | ||
); | ||
$stockItemResource->getConnection()->insertOnDuplicate($entityTable, $stockImportData); | ||
} catch (\Exception $e) { | ||
$this->logger->error($e->getMessage()); | ||
throw new CouldNotSaveException(__('Invalid Stock data for insert'), $e); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
<?php | ||
/** | ||
* Copyright © Magento, Inc. All rights reserved. | ||
* See COPYING.txt for license details. | ||
*/ | ||
declare(strict_types=1); | ||
|
||
namespace Magento\CatalogImportExport\Model; | ||
|
||
/** | ||
* Interface StockItemImporterInterface | ||
* | ||
* @api | ||
*/ | ||
interface StockItemImporterInterface | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there is no default implementation of this interface in CE, just in MSI that will lead to Fatal Error There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
{ | ||
/** | ||
* Handle Import of Stock Item Data | ||
* | ||
* @param array $stockData | ||
* @return void | ||
* @throws \Magento\Framework\Exception\CouldNotSaveException | ||
* @throws \Magento\Framework\Exception\InputException | ||
* @throws \Magento\Framework\Validation\ValidationException | ||
*/ | ||
public function import(array $stockData); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
<?php | ||
/** | ||
* Copyright © Magento, Inc. All rights reserved. | ||
* See COPYING.txt for license details. | ||
*/ | ||
declare(strict_types=1); | ||
|
||
namespace Magento\CatalogInventory\Api; | ||
|
||
use Magento\CatalogInventory\Api\Data\StockItemInterface; | ||
use Magento\Framework\Exception\LocalizedException; | ||
|
||
/** | ||
* @api | ||
*/ | ||
interface RegisterProductSaleInterface | ||
{ | ||
/** | ||
* Subtract product qtys from stock | ||
* Return array of items that require full save | ||
* | ||
* Method signature is unchanged for backward compatibility | ||
* | ||
* @param float[] $items | ||
* @param int $websiteId | ||
* @return StockItemInterface[] | ||
* @throws LocalizedException | ||
*/ | ||
public function registerProductsSale($items, $websiteId = null); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
<?php | ||
/** | ||
* Copyright © Magento, Inc. All rights reserved. | ||
* See COPYING.txt for license details. | ||
*/ | ||
declare(strict_types=1); | ||
|
||
namespace Magento\CatalogInventory\Api; | ||
|
||
/** | ||
* @api | ||
*/ | ||
interface RevertProductSaleInterface | ||
{ | ||
/** | ||
* Revert register product sale | ||
* | ||
* Method signature is unchanged for backward compatibility | ||
* | ||
* @param string[] $items | ||
* @param int $websiteId | ||
* @return bool | ||
*/ | ||
public function revertProductsSale($items, $websiteId = null); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class seems to be unused in the current implementation.
I would suggest delivering it with the code which actually uses it and provides functional test coverage or with unit/integration test which would ensure that it is not broken in the future.