Skip to content

Commit 06c96e9

Browse files
kandykandy
committed
Merge pull request #506 from magento-dragons/PR
[Dragons] Bugfixes
2 parents 1f7a63d + 61814b0 commit 06c96e9

File tree

11 files changed

+138
-89
lines changed

11 files changed

+138
-89
lines changed

‎app/code/Magento/Catalog/Model/ProductRepository.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Magento\Framework\Api\ImageProcessorInterface;
1616
use Magento\Framework\Api\SortOrder;
1717
use Magento\Framework\Exception\InputException;
18+
use Magento\Framework\Exception\LocalizedException;
1819
use Magento\Framework\Exception\NoSuchEntityException;
1920
use Magento\Framework\Exception\StateException;
2021
use Magento\Framework\Exception\ValidatorException;
@@ -546,6 +547,8 @@ public function save(\Magento\Catalog\Api\Data\ProductInterface $product, $saveO
546547
);
547548
} catch (ValidatorException $e) {
548549
throw new CouldNotSaveException(__($e->getMessage()));
550+
} catch (LocalizedException $e) {
551+
throw $e;
549552
} catch (\Exception $e) {
550553
throw new \Magento\Framework\Exception\CouldNotSaveException(__('Unable to save product'));
551554
}

‎app/code/Magento/CatalogSearch/Model/Indexer/Fulltext/Plugin/Product.php

Lines changed: 39 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -6,45 +6,58 @@
66

77
namespace Magento\CatalogSearch\Model\Indexer\Fulltext\Plugin;
88

9+
use Magento\Catalog\Model\ResourceModel\Product as ResourceProduct;
10+
use Magento\Framework\Model\AbstractModel;
11+
912
class Product extends AbstractPlugin
1013
{
1114
/**
1215
* Reindex on product save
1316
*
14-
* @param \Magento\Catalog\Model\ResourceModel\Product $productResource
17+
* @param ResourceProduct $productResource
1518
* @param \Closure $proceed
16-
* @param \Magento\Framework\Model\AbstractModel $product
17-
* @return \Magento\Catalog\Model\ResourceModel\Product
18-
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
19+
* @param AbstractModel $product
20+
* @return ResourceProduct
1921
*/
20-
public function aroundSave(
21-
\Magento\Catalog\Model\ResourceModel\Product $productResource,
22-
\Closure $proceed,
23-
\Magento\Framework\Model\AbstractModel $product
24-
) {
25-
$productResource->addCommitCallback(function () use ($product) {
26-
$this->reindexRow($product->getEntityId());
27-
});
28-
return $proceed($product);
22+
public function aroundSave(ResourceProduct $productResource, \Closure $proceed, AbstractModel $product)
23+
{
24+
return $this->addCommitCallback($productResource, $proceed, $product);
2925
}
3026

3127
/**
3228
* Reindex on product delete
3329
*
34-
* @param \Magento\Catalog\Model\ResourceModel\Product $productResource
30+
* @param ResourceProduct $productResource
3531
* @param \Closure $proceed
36-
* @param \Magento\Framework\Model\AbstractModel $product
37-
* @return \Magento\Catalog\Model\ResourceModel\Product
38-
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
32+
* @param AbstractModel $product
33+
* @return ResourceProduct
3934
*/
40-
public function aroundDelete(
41-
\Magento\Catalog\Model\ResourceModel\Product $productResource,
42-
\Closure $proceed,
43-
\Magento\Framework\Model\AbstractModel $product
44-
) {
45-
$productResource->addCommitCallback(function () use ($product) {
46-
$this->reindexRow($product->getEntityId());
47-
});
48-
return $proceed($product);
35+
public function aroundDelete(ResourceProduct $productResource, \Closure $proceed, AbstractModel $product)
36+
{
37+
return $this->addCommitCallback($productResource, $proceed, $product);
38+
}
39+
40+
/**
41+
* @param ResourceProduct $productResource
42+
* @param \Closure $proceed
43+
* @param AbstractModel $product
44+
* @return ResourceProduct
45+
* @throws \Exception
46+
*/
47+
private function addCommitCallback(ResourceProduct $productResource, \Closure $proceed, AbstractModel $product)
48+
{
49+
try {
50+
$productResource->beginTransaction();
51+
$result = $proceed($product);
52+
$productResource->addCommitCallback(function () use ($product) {
53+
$this->reindexRow($product->getEntityId());
54+
});
55+
$productResource->commit();
56+
} catch (\Exception $e) {
57+
$productResource->rollBack();
58+
throw $e;
59+
}
60+
61+
return $result;
4962
}
5063
}

‎app/code/Magento/CatalogSearch/Test/Unit/Model/Indexer/Fulltext/Plugin/ProductTest.php

Lines changed: 33 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -6,22 +6,28 @@
66

77
namespace Magento\CatalogSearch\Test\Unit\Model\Indexer\Fulltext\Plugin;
88

9+
use Magento\Catalog\Model\Product as ProductModel;
10+
use Magento\Catalog\Model\ResourceModel\Product as ProductResourceModel;
911
use \Magento\CatalogSearch\Model\Indexer\Fulltext\Plugin\Product;
12+
use Magento\Framework\DB\Adapter\AdapterInterface;
13+
use Magento\Framework\Indexer\IndexerInterface;
14+
use Magento\Framework\Indexer\IndexerRegistry;
15+
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
1016

1117
class ProductTest extends \PHPUnit_Framework_TestCase
1218
{
1319
/**
14-
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\Indexer\IndexerInterface
20+
* @var \PHPUnit_Framework_MockObject_MockObject|IndexerInterface
1521
*/
1622
protected $indexerMock;
1723

1824
/**
19-
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Catalog\Model\ResourceModel\Product
25+
* @var \PHPUnit_Framework_MockObject_MockObject|ProductResourceModel
2026
*/
2127
protected $subjectMock;
2228

2329
/**
24-
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Catalog\Model\Product
30+
* @var \PHPUnit_Framework_MockObject_MockObject|ProductModel
2531
*/
2632
protected $productMock;
2733

@@ -31,7 +37,7 @@ class ProductTest extends \PHPUnit_Framework_TestCase
3137
protected $proceed;
3238

3339
/**
34-
* @var \Magento\Framework\Indexer\IndexerRegistry|\PHPUnit_Framework_MockObject_MockObject
40+
* @var IndexerRegistry|\PHPUnit_Framework_MockObject_MockObject
3541
*/
3642
protected $indexerRegistryMock;
3743

@@ -42,30 +48,34 @@ class ProductTest extends \PHPUnit_Framework_TestCase
4248

4349
protected function setUp()
4450
{
45-
$this->productMock = $this->getMock('Magento\Catalog\Model\Product', [], [], '', false);
46-
$this->subjectMock = $this->getMock('Magento\Catalog\Model\ResourceModel\Product', [], [], '', false);
47-
$this->indexerMock = $this->getMockForAbstractClass(
48-
'Magento\Framework\Indexer\IndexerInterface',
49-
[],
50-
'',
51-
false,
52-
false,
53-
true,
54-
['getId', 'getState', '__wakeup']
55-
);
56-
$this->indexerRegistryMock = $this->getMock(
57-
'Magento\Framework\Indexer\IndexerRegistry',
58-
['get'],
59-
[],
60-
'',
61-
false
62-
);
51+
$this->productMock = $this->getMockBuilder(ProductModel::class)
52+
->disableOriginalConstructor()
53+
->getMock();
54+
$this->subjectMock = $this->getMockBuilder(ProductResourceModel::class)
55+
->disableOriginalConstructor()
56+
->getMock();
57+
$connection = $this->getMockBuilder(AdapterInterface::class)
58+
->disableOriginalConstructor()
59+
->getMockForAbstractClass();
60+
$this->subjectMock->method('getConnection')->willReturn($connection);
61+
62+
$this->indexerMock = $this->getMockBuilder(IndexerInterface::class)
63+
->disableOriginalConstructor()
64+
->setMethods(['getId', 'getState', '__wakeup'])
65+
->getMockForAbstractClass();
66+
$this->indexerRegistryMock = $this->getMockBuilder(IndexerRegistry::class)
67+
->disableOriginalConstructor()
68+
->setMethods(['get'])
69+
->getMock();
6370

6471
$this->proceed = function () {
6572
return $this->subjectMock;
6673
};
6774

68-
$this->model = new Product($this->indexerRegistryMock);
75+
$this->model = (new ObjectManager($this))->getObject(
76+
Product::class,
77+
['indexerRegistry' => $this->indexerRegistryMock]
78+
);
6979
}
7080

7181
public function testAfterSaveNonScheduled()

‎app/code/Magento/GoogleOptimizer/Observer/AbstractSave.php

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public function execute(Observer $observer)
5858
{
5959
$this->_initEntity($observer);
6060

61-
if ($this->_isGoogleExperimentActive()) {
61+
if ($this->_isGoogleExperimentActive() && $this->isDataAvailable()) {
6262
$this->_processCode();
6363
}
6464

@@ -112,11 +112,10 @@ protected function _processCode()
112112
*/
113113
protected function _initRequestParams()
114114
{
115-
$params = $this->_request->getParam('google_experiment');
116-
if (!is_array($params) || !isset($params['experiment_script']) || !isset($params['code_id'])) {
115+
if (!$this->isDataAvailable()) {
117116
throw new \InvalidArgumentException('Wrong request parameters');
118117
}
119-
$this->_params = $params;
118+
$this->_params = $this->getRequestData();
120119
}
121120

122121
/**
@@ -181,4 +180,21 @@ protected function _deleteCode()
181180
{
182181
$this->_modelCode->delete();
183182
}
183+
184+
/**
185+
* @return array
186+
*/
187+
private function isDataAvailable()
188+
{
189+
$params = $this->getRequestData();
190+
return is_array($params) && isset($params['experiment_script']) && isset($params['code_id']);
191+
}
192+
193+
/**
194+
* @return mixed
195+
*/
196+
private function getRequestData()
197+
{
198+
return $this->_request->getParam('google_experiment');
199+
}
184200
}

‎app/code/Magento/GoogleOptimizer/Test/Unit/Observer/Category/SaveGoogleExperimentScriptObserverTest.php

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ public function testCreatingCodeIfRequestIsValid()
8585
);
8686

8787
$this->_requestMock->expects(
88-
$this->once()
88+
$this->exactly(3)
8989
)->method(
9090
'getParam'
9191
)->with(
@@ -113,8 +113,6 @@ public function testCreatingCodeIfRequestIsValid()
113113

114114
/**
115115
* @param array $params
116-
* @expectedException \InvalidArgumentException
117-
* @expectedExceptionMessage Wrong request parameters
118116
* @dataProvider dataProviderWrongRequestForCreating
119117
*/
120118
public function testCreatingCodeIfRequestIsNotValid($params)
@@ -174,7 +172,7 @@ public function testEditingCodeIfRequestIsValid()
174172
);
175173

176174
$this->_requestMock->expects(
177-
$this->once()
175+
$this->exactly(3)
178176
)->method(
179177
'getParam'
180178
)->with(
@@ -223,7 +221,7 @@ public function testEditingCodeIfCodeModelIsNotFound()
223221
);
224222

225223
$this->_requestMock->expects(
226-
$this->once()
224+
$this->exactly(3)
227225
)->method(
228226
'getParam'
229227
)->with(
@@ -254,7 +252,7 @@ public function testRemovingCode()
254252
);
255253

256254
$this->_requestMock->expects(
257-
$this->once()
255+
$this->exactly(3)
258256
)->method(
259257
'getParam'
260258
)->with(

‎app/code/Magento/GoogleOptimizer/Test/Unit/Observer/CmsPage/SaveGoogleExperimentScriptObserverTest.php

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ public function testCreatingCodeIfRequestIsValid()
7070
$this->_helperMock->expects($this->once())->method('isGoogleExperimentActive')->will($this->returnValue(true));
7171

7272
$this->_requestMock->expects(
73-
$this->once()
73+
$this->exactly(3)
7474
)->method(
7575
'getParam'
7676
)->with(
@@ -98,8 +98,6 @@ public function testCreatingCodeIfRequestIsValid()
9898

9999
/**
100100
* @param array $params
101-
* @expectedException \InvalidArgumentException
102-
* @expectedExceptionMessage Wrong request parameters
103101
* @dataProvider dataProviderWrongRequestForCreating
104102
*/
105103
public function testCreatingCodeIfRequestIsNotValid($params)
@@ -143,7 +141,7 @@ public function testEditingCodeIfRequestIsValid()
143141
$this->_helperMock->expects($this->once())->method('isGoogleExperimentActive')->will($this->returnValue(true));
144142

145143
$this->_requestMock->expects(
146-
$this->once()
144+
$this->exactly(3)
147145
)->method(
148146
'getParam'
149147
)->with(
@@ -184,7 +182,7 @@ public function testEditingCodeIfCodeModelIsNotFound()
184182
$this->_helperMock->expects($this->once())->method('isGoogleExperimentActive')->will($this->returnValue(true));
185183

186184
$this->_requestMock->expects(
187-
$this->once()
185+
$this->exactly(3)
188186
)->method(
189187
'getParam'
190188
)->with(
@@ -215,7 +213,7 @@ public function testRemovingCode()
215213
);
216214

217215
$this->_requestMock->expects(
218-
$this->once()
216+
$this->exactly(3)
219217
)->method(
220218
'getParam'
221219
)->with(

‎app/code/Magento/GoogleOptimizer/Test/Unit/Observer/Product/SaveGoogleExperimentScriptObserverTest.php

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ public function testCreatingCodeIfRequestIsValid()
8585
);
8686

8787
$this->_requestMock->expects(
88-
$this->once()
88+
$this->exactly(3)
8989
)->method(
9090
'getParam'
9191
)->with(
@@ -113,8 +113,6 @@ public function testCreatingCodeIfRequestIsValid()
113113

114114
/**
115115
* @param array $params
116-
* @expectedException \InvalidArgumentException
117-
* @expectedExceptionMessage Wrong request parameters
118116
* @dataProvider dataProviderWrongRequestForCreating
119117
*/
120118
public function testCreatingCodeIfRequestIsNotValid($params)
@@ -174,7 +172,7 @@ public function testEditingCodeIfRequestIsValid()
174172
);
175173

176174
$this->_requestMock->expects(
177-
$this->once()
175+
$this->exactly(3)
178176
)->method(
179177
'getParam'
180178
)->with(
@@ -223,7 +221,7 @@ public function testEditingCodeIfCodeModelIsNotFound()
223221
);
224222

225223
$this->_requestMock->expects(
226-
$this->once()
224+
$this->exactly(3)
227225
)->method(
228226
'getParam'
229227
)->with(
@@ -254,7 +252,7 @@ public function testRemovingCode()
254252
);
255253

256254
$this->_requestMock->expects(
257-
$this->once()
255+
$this->exactly(3)
258256
)->method(
259257
'getParam'
260258
)->with(

‎dev/tests/functional/tests/app/Magento/Catalog/Test/Block/Product/Compare/Sidebar.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,5 @@ function () use ($rootElement, $selector) {
7979
}
8080
);
8181
$this->_rootElement->find($this->clearAll)->click();
82-
$this->browser->acceptAlert();
8382
}
8483
}

‎lib/internal/Magento/Framework/Model/ResourceModel/AbstractResource.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ public function beginTransaction()
5656
/**
5757
* Subscribe some callback to transaction commit
5858
*
59-
* @param array $callback
59+
* @param callable|array $callback
6060
* @return $this
6161
* @api
6262
*/

0 commit comments

Comments
 (0)