Skip to content

Commit

Permalink
Merge pull request #425 from magento-south/BUGS
Browse files Browse the repository at this point in the history
[South] Bugfixes
- MAGETWO-58277 Saving category on catalog with 20k+ products is very slow (from 5mins till 1 hour)
- MAGETWO-23764 Category/Product Indexer fails with mysql fatal on large catalog
- MAGETWO-33568 Customer is redirected to "Compare Products" Frontend page if tries to remove a Product from comparing
- MAGETWO-56512 Products created via REST API are not visible after enabling the product via scope = All Store Views
- MAGETWO-55154 [GitHub] IndexerHandlerFactory: Indexer Object cast to String
- MAGETWO-57835 [Github] Cannot save customer dob attribute if admin interface locale is en_GB #6323
- MAGETWO-57629 ACL not used on grid quick edits
  • Loading branch information
slavvka authored Sep 26, 2016
2 parents c5d04c6 + c37e8ce commit 8931b9c
Show file tree
Hide file tree
Showing 31 changed files with 1,414 additions and 282 deletions.
14 changes: 13 additions & 1 deletion app/code/Magento/Catalog/Controller/Adminhtml/Category.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ abstract class Category extends \Magento\Backend\App\Action
*/
protected function _initCategory($getRootInstead = false)
{
$categoryId = (int)$this->getRequest()->getParam('id', false);
$categoryId = $this->resolveCategoryId();
$storeId = (int)$this->getRequest()->getParam('store');
$category = $this->_objectManager->create(\Magento\Catalog\Model\Category::class);
$category->setStoreId($storeId);
Expand Down Expand Up @@ -62,6 +62,18 @@ protected function _initCategory($getRootInstead = false)
return $category;
}

/**
* Resolve Category Id (from get or from post)
*
* @return int
*/
private function resolveCategoryId()
{
$categoryId = (int)$this->getRequest()->getParam('id', false);

return $categoryId ?: (int)$this->getRequest()->getParam('entity_id', false);
}

/**
* Build response for ajax request
*
Expand Down
8 changes: 3 additions & 5 deletions app/code/Magento/Catalog/Helper/Product/Compare.php
Original file line number Diff line number Diff line change
Expand Up @@ -228,10 +228,9 @@ public function getRemoveUrl()
*/
public function getPostDataRemove($product)
{
$listCleanUrl = $this->getEncodedUrl($this->_getUrl('catalog/product_compare'));
$data = [
\Magento\Framework\App\ActionInterface::PARAM_NAME_URL_ENCODED => $listCleanUrl,
'product' => $product->getId()
\Magento\Framework\App\ActionInterface::PARAM_NAME_URL_ENCODED => '',
'product' => $product->getId(),
];
return $this->postHelper->getPostData($this->getRemoveUrl(), $data);
}
Expand All @@ -253,9 +252,8 @@ public function getClearListUrl()
*/
public function getPostDataClearList()
{
$refererUrl = $this->_getRequest()->getServer('HTTP_REFERER');
$params = [
\Magento\Framework\App\ActionInterface::PARAM_NAME_URL_ENCODED => $this->urlEncoder->encode($refererUrl)
\Magento\Framework\App\ActionInterface::PARAM_NAME_URL_ENCODED => '',
];
return $this->postHelper->getPostData($this->getClearListUrl(), $params);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@

class Full extends \Magento\Catalog\Model\Indexer\Category\Product\AbstractAction
{
/**
* Whether to use main or temporary index table
*
* @var bool
*/
protected $useTempTable = false;

/**
* Refresh entities index
*
Expand Down
19 changes: 13 additions & 6 deletions app/code/Magento/Catalog/Model/ProductRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -313,15 +313,22 @@ protected function initializeProductData(array $productData, $createNew)
*/
private function assignProductToWebsites(\Magento\Catalog\Model\Product $product)
{
$websiteIds = $product->getWebsiteIds();

if (!$this->storeManager->hasSingleStore()) {
if ($this->storeManager->getStore()->getCode() == \Magento\Store\Model\Store::ADMIN_CODE) {
$websiteIds = array_keys($this->storeManager->getWebsites());
} else {
$websiteIds = [$this->storeManager->getStore()->getWebsiteId()];
}
$websiteIds = array_unique(
array_merge(
$websiteIds,
[$this->storeManager->getStore()->getWebsiteId()]
)
);
}

$product->setWebsiteIds(array_unique(array_merge($product->getWebsiteIds(), $websiteIds)));
if ($this->storeManager->getStore(true)->getCode() == \Magento\Store\Model\Store::ADMIN_CODE) {
$websiteIds = array_keys($this->storeManager->getWebsites());
}

$product->setWebsiteIds($websiteIds);
}

/**
Expand Down
44 changes: 28 additions & 16 deletions app/code/Magento/Catalog/Model/ResourceModel/Category.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ class Category extends AbstractResource
*/
protected $_categoryProductTable;

/**
* @var array[]
*/
private $entitiesWhereAttributesIs;

/**
* Id of 'is_active' category attribute
*
Expand Down Expand Up @@ -573,22 +578,29 @@ public function getIsActiveAttributeId()
*/
public function findWhereAttributeIs($entityIdsFilter, $attribute, $expectedValue)
{
$linkField = $this->getLinkField();
$bind = ['attribute_id' => $attribute->getId(), 'value' => $expectedValue];
$selectEntities = $this->getConnection()->select()->from(
['ce' => $this->getTable('catalog_category_entity')],
['entity_id']
)->joinLeft(
['ci' => $attribute->getBackend()->getTable()],
"ci.{$linkField} = ce.{$linkField} AND attribute_id = :attribute_id",
['value']
)->where(
'ci.value = :value'
)->where(
'ce.entity_id IN (?)',
$entityIdsFilter
);
return $this->getConnection()->fetchCol($selectEntities, $bind);
$entityIdsFilterHash = md5(serialize($entityIdsFilter));

if (!isset($this->entitiesWhereAttributesIs[$entityIdsFilterHash][$attribute->getId()][$expectedValue])) {
$linkField = $this->getLinkField();
$bind = ['attribute_id' => $attribute->getId(), 'value' => $expectedValue];
$selectEntities = $this->getConnection()->select()->from(
['ce' => $this->getTable('catalog_category_entity')],
['entity_id']
)->joinLeft(
['ci' => $attribute->getBackend()->getTable()],
"ci.{$linkField} = ce.{$linkField} AND attribute_id = :attribute_id",
['value']
)->where(
'ci.value = :value'
)->where(
'ce.entity_id IN (?)',
$entityIdsFilter
);
$this->entitiesWhereAttributesIs[$entityIdsFilterHash][$attribute->getId()][$expectedValue] =
$this->getConnection()->fetchCol($selectEntities, $bind);
}

return $this->entitiesWhereAttributesIs[$entityIdsFilterHash][$attribute->getId()][$expectedValue];
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@

use Magento\Framework\App\ResourceConnection;

/**
* @deprecated
*/
class MaxHeapTableSizeProcessor
{
/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
use Magento\Catalog\Model\ResourceModel\MaxHeapTableSizeProcessor;
use Psr\Log\LoggerInterface;

/**
* @deprecated
*/
class MaxHeapTableSizeProcessorOnFullReindex
{
/**
Expand Down
17 changes: 3 additions & 14 deletions app/code/Magento/Catalog/Test/Unit/Helper/Product/CompareTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -113,18 +113,13 @@ public function testGetPostDataRemove()
//Data
$productId = 1;
$removeUrl = 'catalog/product_compare/remove';
$compareListUrl = 'catalog/product_compare';
$postParams = [
Action::PARAM_NAME_URL_ENCODED => strtr(base64_encode($compareListUrl), '+/=', '-_,'),
Action::PARAM_NAME_URL_ENCODED => '',
'product' => $productId
];

//Verification
$this->urlBuilder->expects($this->at(0))
->method('getUrl')
->with($compareListUrl)
->will($this->returnValue($compareListUrl));
$this->urlBuilder->expects($this->at(1))
$this->urlBuilder->expects($this->once())
->method('getUrl')
->with($removeUrl)
->will($this->returnValue($removeUrl));
Expand Down Expand Up @@ -159,18 +154,12 @@ public function testGetClearListUrl()
public function testGetPostDataClearList()
{
//Data
$refererUrl = 'home/';
$clearUrl = 'catalog/product_compare/clear';
$postParams = [
Action::PARAM_NAME_URL_ENCODED => strtr(base64_encode($refererUrl), '+/=', '-_,')
Action::PARAM_NAME_URL_ENCODED => ''
];

//Verification
$this->request->expects($this->once())
->method('getServer')
->with('HTTP_REFERER')
->will($this->returnValue($refererUrl));

$this->urlBuilder->expects($this->once())
->method('getUrl')
->with($clearUrl)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use Magento\Framework\DB\Adapter\ConnectionException;
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
use Magento\Framework\Api\SearchCriteria\CollectionProcessorInterface;
use Magento\Store\Api\Data\StoreInterface;

/**
* Class ProductRepositoryTest
Expand Down Expand Up @@ -284,7 +285,6 @@ protected function setUp()
$storeMock->expects($this->any())->method('getWebsiteId')->willReturn('1');
$storeMock->expects($this->any())->method('getCode')->willReturn(\Magento\Store\Model\Store::ADMIN_CODE);
$this->storeManagerMock->expects($this->any())->method('getStore')->willReturn($storeMock);
$this->storeManagerMock->expects($this->any())->method('getWebsites')->willReturn([1 => 'default']);

$this->mediaGalleryProcessor = $this->getMock(
\Magento\Catalog\Model\Product\Gallery\Processor::class,
Expand Down Expand Up @@ -495,6 +495,7 @@ public function testGetBySkuFromCacheInitializedInGetById()

public function testSaveExisting()
{
$this->storeManagerMock->expects($this->any())->method('getWebsites')->willReturn([1 => 'default']);
$this->resourceModelMock->expects($this->any())->method('getIdBySku')->will($this->returnValue(100));
$this->productFactoryMock->expects($this->any())
->method('create')
Expand All @@ -514,6 +515,7 @@ public function testSaveExisting()

public function testSaveNew()
{
$this->storeManagerMock->expects($this->any())->method('getWebsites')->willReturn([1 => 'default']);
$this->resourceModelMock->expects($this->at(0))->method('getIdBySku')->will($this->returnValue(null));
$this->resourceModelMock->expects($this->at(3))->method('getIdBySku')->will($this->returnValue(100));
$this->productFactoryMock->expects($this->any())
Expand All @@ -538,6 +540,7 @@ public function testSaveNew()
*/
public function testSaveUnableToSaveException()
{
$this->storeManagerMock->expects($this->any())->method('getWebsites')->willReturn([1 => 'default']);
$this->resourceModelMock->expects($this->exactly(1))->method('getIdBySku')->will($this->returnValue(null));
$this->productFactoryMock->expects($this->exactly(2))
->method('create')
Expand All @@ -562,6 +565,7 @@ public function testSaveUnableToSaveException()
*/
public function testSaveException()
{
$this->storeManagerMock->expects($this->any())->method('getWebsites')->willReturn([1 => 'default']);
$this->resourceModelMock->expects($this->exactly(1))->method('getIdBySku')->will($this->returnValue(null));
$this->productFactoryMock->expects($this->exactly(2))
->method('create')
Expand All @@ -587,6 +591,7 @@ public function testSaveException()
*/
public function testSaveInvalidProductException()
{
$this->storeManagerMock->expects($this->any())->method('getWebsites')->willReturn([1 => 'default']);
$this->resourceModelMock->expects($this->exactly(1))->method('getIdBySku')->will($this->returnValue(null));
$this->productFactoryMock->expects($this->exactly(2))
->method('create')
Expand All @@ -610,6 +615,7 @@ public function testSaveInvalidProductException()
*/
public function testSaveThrowsTemporaryStateExceptionIfDatabaseConnectionErrorOccurred()
{
$this->storeManagerMock->expects($this->any())->method('getWebsites')->willReturn([1 => 'default']);
$this->productFactoryMock->expects($this->any())
->method('create')
->will($this->returnValue($this->productMock));
Expand Down Expand Up @@ -796,6 +802,7 @@ public function cacheKeyDataProvider()
*/
public function testSaveExistingWithOptions(array $newOptions, array $existingOptions, array $expectedData)
{
$this->storeManagerMock->expects($this->any())->method('getWebsites')->willReturn([1 => 'default']);
$this->resourceModelMock->expects($this->any())->method('getIdBySku')->will($this->returnValue(100));
$this->productFactoryMock->expects($this->any())
->method('create')
Expand Down Expand Up @@ -964,6 +971,7 @@ public function saveExistingWithOptionsDataProvider()
*/
public function testSaveWithLinks(array $newLinks, array $existingLinks, array $expectedData)
{
$this->storeManagerMock->expects($this->any())->method('getWebsites')->willReturn([1 => 'default']);
$this->resourceModelMock->expects($this->any())->method('getIdBySku')->will($this->returnValue(100));
$this->productFactoryMock->expects($this->any())
->method('create')
Expand Down Expand Up @@ -1143,6 +1151,7 @@ protected function setupProductMocksForSave()

public function testSaveExistingWithNewMediaGalleryEntries()
{
$this->storeManagerMock->expects($this->any())->method('getWebsites')->willReturn([1 => 'default']);
$newEntriesData = [
[
"label" => "label_text",
Expand Down Expand Up @@ -1222,8 +1231,48 @@ public function testSaveExistingWithNewMediaGalleryEntries()
$this->model->save($this->productMock);
}

public function websitesProvider()
{
return [
[[1,2,3]]
];
}

public function testSaveWithDifferentWebsites()
{
$storeMock = $this->getMock(StoreInterface::class);
$this->resourceModelMock->expects($this->at(0))->method('getIdBySku')->will($this->returnValue(null));
$this->resourceModelMock->expects($this->at(3))->method('getIdBySku')->will($this->returnValue(100));
$this->productFactoryMock->expects($this->any())
->method('create')
->will($this->returnValue($this->productMock));
$this->initializationHelperMock->expects($this->never())->method('initialize');
$this->resourceModelMock->expects($this->once())->method('validate')->with($this->productMock)
->willReturn(true);
$this->resourceModelMock->expects($this->once())->method('save')->with($this->productMock)->willReturn(true);
$this->extensibleDataObjectConverterMock
->expects($this->once())
->method('toNestedArray')
->will($this->returnValue($this->productData));
$this->storeManagerMock->expects($this->any())
->method('getStore')
->willReturn($storeMock);
$this->storeManagerMock->expects($this->once())
->method('getWebsites')
->willReturn([
1 => ['first'],
2 => ['second'],
3 => ['third']
]);
$this->productMock->expects($this->once())->method('getWebsiteIds')->willReturn([1,2,3]);
$this->productMock->expects($this->once())->method('setWebsiteIds')->willReturn([2,3]);

$this->assertEquals($this->productMock, $this->model->save($this->productMock));
}

public function testSaveExistingWithMediaGalleryEntries()
{
$this->storeManagerMock->expects($this->any())->method('getWebsites')->willReturn([1 => 'default']);
//update one entry, delete one entry
$newEntries = [
[
Expand Down
1 change: 0 additions & 1 deletion app/code/Magento/Catalog/etc/adminhtml/di.xml
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@
</type>
<type name="Magento\Catalog\Model\Indexer\Category\Product\Action\Full">
<plugin name="invalidate_pagecache_after_full_reindex" type="Magento\Catalog\Plugin\Model\Indexer\Category\Product\Execute" />
<plugin name="max_heap_tablse_size_processor_on_full_reindex" type="Magento\Catalog\Plugin\Model\Indexer\Category\Product\MaxHeapTableSizeProcessorOnFullReindex"/>
</type>
<type name="Magento\Catalog\Model\ResourceModel\Attribute">
<plugin name="invalidate_pagecache_after_attribute_save" type="Magento\Catalog\Plugin\Model\ResourceModel\Attribute\Save" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,14 @@ public function create(array $data = [])
$indexer = $this->_objectManager->create($this->handlers[$currentHandler], $data);

if (!$indexer instanceof IndexerInterface) {
throw new \InvalidArgumentException($indexer . ' doesn\'t implement \Magento\Framework\IndexerInterface');
throw new \InvalidArgumentException(
$currentHandler . ' indexer handler doesn\'t implement ' . IndexerInterface::class
);
}

if ($indexer && !$indexer->isAvailable()) {
throw new \LogicException(
'Indexer handler is not available: ' . $indexer
'Indexer handler is not available: ' . $currentHandler
);
}
return $indexer;
Expand Down
Loading

0 comments on commit 8931b9c

Please sign in to comment.