Skip to content

Commit 0383b22

Browse files
authored
Merge pull request #2774 from magento-panda/MAGETWO-87589
Fixed issues: - MAGETWO-87589: [Magento Cloud] - Issue with polluted database when updating product attributes through API
2 parents b33721c + 4fd3d67 commit 0383b22

File tree

19 files changed

+587
-109
lines changed

19 files changed

+587
-109
lines changed

app/code/Magento/CatalogUrlRewrite/Model/ProductUrlPathGenerator.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,11 +120,12 @@ public function getCanonicalUrlPath($product, $category = null)
120120
* Generate product url key based on url_key entered by merchant or product name
121121
*
122122
* @param \Magento\Catalog\Model\Product $product
123-
* @return string
123+
* @return string|null
124124
*/
125125
public function getUrlKey($product)
126126
{
127-
return $product->getUrlKey() === false ? false : $this->prepareProductUrlKey($product);
127+
$generatedProductUrlKey = $this->prepareProductUrlKey($product);
128+
return ($product->getUrlKey() === false || empty($generatedProductUrlKey)) ? null : $generatedProductUrlKey;
128129
}
129130

130131
/**

app/code/Magento/CatalogUrlRewrite/Observer/ProductUrlKeyAutogeneratorObserver.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ public function execute(\Magento\Framework\Event\Observer $observer)
3333
{
3434
/** @var Product $product */
3535
$product = $observer->getEvent()->getProduct();
36-
$product->setUrlKey($this->productUrlPathGenerator->getUrlKey($product));
36+
$urlKey = $this->productUrlPathGenerator->getUrlKey($product);
37+
if (null !== $urlKey) {
38+
$product->setUrlKey($urlKey);
39+
}
3740
}
3841
}

app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/ProductUrlPathGeneratorTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ public function testGetUrlKey($productUrlKey, $expectedUrlKey)
105105
{
106106
$this->product->expects($this->any())->method('getUrlKey')->will($this->returnValue($productUrlKey));
107107
$this->product->expects($this->any())->method('formatUrlKey')->will($this->returnValue($productUrlKey));
108-
$this->assertEquals($expectedUrlKey, $this->productUrlPathGenerator->getUrlKey($this->product));
108+
$this->assertSame($expectedUrlKey, $this->productUrlPathGenerator->getUrlKey($this->product));
109109
}
110110

111111
/**
@@ -114,7 +114,7 @@ public function testGetUrlKey($productUrlKey, $expectedUrlKey)
114114
public function getUrlKeyDataProvider()
115115
{
116116
return [
117-
'URL Key use default' => [false, false],
117+
'URL Key use default' => [false, null],
118118
'URL Key empty' => ['product-url', 'product-url'],
119119
];
120120
}
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
namespace Magento\CatalogUrlRewrite\Test\Unit\Observer;
7+
8+
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager as ObjectManagerHelper;
9+
use \Magento\CatalogUrlRewrite\Model\ProductUrlPathGenerator;
10+
11+
/**
12+
* Unit tests for \Magento\CatalogUrlRewrite\Observer\ProductUrlKeyAutogeneratorObserver class
13+
*/
14+
class ProductUrlKeyAutogeneratorObserverTest extends \PHPUnit\Framework\TestCase
15+
{
16+
/**
17+
* @var \Magento\CatalogUrlRewrite\Model\ProductUrlPathGenerator|\PHPUnit_Framework_MockObject_MockObject
18+
*/
19+
private $productUrlPathGenerator;
20+
21+
/** @var \Magento\CatalogUrlRewrite\Observer\ProductUrlKeyAutogeneratorObserver */
22+
private $productUrlKeyAutogeneratorObserver;
23+
24+
/**
25+
* @inheritdoc
26+
*/
27+
protected function setUp()
28+
{
29+
$this->productUrlPathGenerator = $this->getMockBuilder(ProductUrlPathGenerator::class)
30+
->disableOriginalConstructor()
31+
->setMethods(['getUrlKey'])
32+
->getMock();
33+
34+
$this->productUrlKeyAutogeneratorObserver = (new ObjectManagerHelper($this))->getObject(
35+
\Magento\CatalogUrlRewrite\Observer\ProductUrlKeyAutogeneratorObserver::class,
36+
[
37+
'productUrlPathGenerator' => $this->productUrlPathGenerator
38+
]
39+
);
40+
}
41+
42+
public function testExecuteWithUrlKey()
43+
{
44+
$urlKey = 'product_url_key';
45+
46+
$product = $this->getMockBuilder(\Magento\Catalog\Model\Product::class)
47+
->disableOriginalConstructor()
48+
->setMethods(['setUrlKey'])
49+
->getMock();
50+
$product->expects($this->atLeastOnce())->method('setUrlKey')->with($urlKey);
51+
$event = $this->getMockBuilder(\Magento\Framework\Event::class)
52+
->disableOriginalConstructor()
53+
->setMethods(['getProduct'])
54+
->getMock();
55+
$event->expects($this->atLeastOnce())->method('getProduct')->willReturn($product);
56+
/** @var \Magento\Framework\Event\Observer|\PHPUnit_Framework_MockObject_MockObject $observer */
57+
$observer = $this->getMockBuilder(\Magento\Framework\Event\Observer::class)
58+
->disableOriginalConstructor()
59+
->setMethods(['getEvent'])
60+
->getMock();
61+
$observer->expects($this->atLeastOnce())->method('getEvent')->willReturn($event);
62+
$this->productUrlPathGenerator->expects($this->atLeastOnce())->method('getUrlKey')->with($product)
63+
->willReturn($urlKey);
64+
65+
$this->productUrlKeyAutogeneratorObserver->execute($observer);
66+
}
67+
68+
public function testExecuteWithEmptyUrlKey()
69+
{
70+
$product = $this->getMockBuilder(\Magento\Catalog\Model\Product::class)
71+
->disableOriginalConstructor()
72+
->setMethods(['setUrlKey'])
73+
->getMock();
74+
$product->expects($this->never())->method('setUrlKey');
75+
$event = $this->getMockBuilder(\Magento\Framework\Event::class)
76+
->disableOriginalConstructor()
77+
->setMethods(['getProduct'])
78+
->getMock();
79+
$event->expects($this->atLeastOnce())->method('getProduct')->willReturn($product);
80+
/** @var \Magento\Framework\Event\Observer|\PHPUnit_Framework_MockObject_MockObject $observer */
81+
$observer = $this->getMockBuilder(\Magento\Framework\Event\Observer::class)
82+
->disableOriginalConstructor()
83+
->setMethods(['getEvent'])
84+
->getMock();
85+
$observer->expects($this->atLeastOnce())->method('getEvent')->willReturn($event);
86+
$this->productUrlPathGenerator->expects($this->atLeastOnce())->method('getUrlKey')->with($product)
87+
->willReturn(null);
88+
89+
$this->productUrlKeyAutogeneratorObserver->execute($observer);
90+
}
91+
}

app/code/Magento/Ui/view/base/web/js/form/element/abstract.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -450,6 +450,10 @@ define([
450450
*/
451451
toggleUseDefault: function (state) {
452452
this.disabled(state);
453+
454+
if (this.source && this.hasService()) {
455+
this.source.set('data.use_default.' + this.index, Number(state));
456+
}
453457
},
454458

455459
/**

app/code/Magento/Ui/view/base/web/js/form/element/single-checkbox-use-config.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ define([
3636
*/
3737
toggleElement: function () {
3838
this.disabled(this.isUseDefault() || this.isUseConfig());
39+
40+
if (this.source) {
41+
this.source.set('data.use_default.' + this.index, Number(this.isUseDefault()));
42+
}
3943
}
4044
});
4145
});

app/code/Magento/Ui/view/base/web/templates/form/element/helper/service.html

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
attr="
1111
id: $data.uid + '_default',
1212
name: 'use_default[' + $data.index + ']',
13-
'data-form-part': $data.ns
1413
"
1514
ko-checked="isUseDefault"
1615
ko-disabled="$data.serviceDisabled">

dev/tests/js/jasmine/tests/app/code/Magento/Ui/base/js/form/element/abstract.test.js

Lines changed: 42 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,50 @@
55

66
/*eslint max-nested-callbacks: 0*/
77
define([
8-
'Magento_Ui/js/form/element/abstract'
9-
], function (Abstract) {
8+
'squire'
9+
], function (Squire) {
1010
'use strict';
1111

1212
describe('Magento_Ui/js/form/element/abstract', function () {
13-
var params, model;
14-
15-
beforeEach(function () {
13+
var injector = new Squire(),
14+
providerMock = {
15+
get: jasmine.createSpy(),
16+
set: jasmine.createSpy()
17+
},
18+
mocks = {
19+
'Magento_Ui/js/lib/registry/registry': {
20+
/** Method stub. */
21+
get: function () {
22+
return providerMock;
23+
},
24+
create: jasmine.createSpy(),
25+
set: jasmine.createSpy(),
26+
async: jasmine.createSpy()
27+
},
28+
'/mage/utils/wrapper': jasmine.createSpy()
29+
},
30+
dataScope = 'abstract',
1631
params = {
17-
dataScope: 'abstract'
18-
};
19-
model = new Abstract(params);
20-
model.source = jasmine.createSpyObj('model.source', ['set']);
32+
provider: 'provName',
33+
name: '',
34+
index: 'testIndex',
35+
dataScope: dataScope,
36+
service: {
37+
template: 'ui/form/element/helper/service'
38+
}
39+
},
40+
model;
41+
42+
beforeEach(function (done) {
43+
injector.mock(mocks);
44+
injector.require([
45+
'Magento_Ui/js/form/element/abstract',
46+
'knockoutjs/knockout-es5'
47+
], function (Constr) {
48+
model = new Constr(params);
49+
50+
done();
51+
});
2152
});
2253

2354
describe('initialize method', function () {
@@ -50,8 +81,10 @@ define([
5081
var expectedValue = 1;
5182

5283
spyOn(model, 'getInitialValue').and.returnValue(expectedValue);
84+
model.service = true;
5385
expect(model.setInitialValue()).toEqual(model);
5486
expect(model.getInitialValue).toHaveBeenCalled();
87+
expect(model.source.set).toHaveBeenCalledWith('data.use_default.' + model.index, 0);
5588
expect(model.value()).toEqual(expectedValue);
5689
});
5790
});

dev/tests/js/jasmine/tests/app/code/Magento/Ui/base/js/form/element/boolean.test.js

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,45 @@
66
/*eslint max-nested-callbacks: 0*/
77

88
define([
9-
'Magento_Ui/js/form/element/boolean'
10-
], function (BooleanElement) {
9+
'squire'
10+
], function (Squire) {
1111
'use strict';
1212

1313
describe('Magento_Ui/js/form/element/boolean', function () {
14-
var params, model;
14+
var injector = new Squire(),
15+
mocks = {
16+
'Magento_Ui/js/lib/registry/registry': {
17+
/** Method stub. */
18+
get: function () {
19+
return {
20+
get: jasmine.createSpy(),
21+
set: jasmine.createSpy()
22+
};
23+
},
24+
create: jasmine.createSpy(),
25+
set: jasmine.createSpy(),
26+
async: jasmine.createSpy()
27+
},
28+
'/mage/utils/wrapper': jasmine.createSpy()
29+
},
30+
model,
31+
dataScope = 'dataScope';
1532

16-
beforeEach(function () {
17-
params = {
18-
dataScope: 'abstract'
19-
};
20-
model = new BooleanElement(params);
33+
beforeEach(function (done) {
34+
injector.mock(mocks);
35+
injector.require([
36+
'Magento_Ui/js/form/element/boolean',
37+
'knockoutjs/knockout-es5'
38+
], function (Constr) {
39+
model = new Constr({
40+
provider: 'provName',
41+
name: '',
42+
index: '',
43+
dataScope: dataScope
44+
});
45+
46+
done();
47+
});
2148
});
2249

2350
describe('getInitialValue method', function () {

dev/tests/js/jasmine/tests/app/code/Magento/Ui/base/js/form/element/date-time.test.js

Lines changed: 44 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,23 +6,52 @@
66
/*eslint max-nested-callbacks: 0*/
77

88
define([
9-
'Magento_Ui/js/form/element/date',
10-
'mageUtils',
11-
'moment'
12-
], function (DateElement, utils, moment) {
9+
'squire'
10+
], function (Squire) {
1311
'use strict';
1412

15-
describe('Magento_Ui/js/form/element/date', function () {
16-
var params, model;
17-
18-
beforeEach(function () {
19-
params = {
20-
dataScope: 'abstract',
21-
options: {
22-
showsTime: true
23-
}
24-
};
25-
model = new DateElement(params);
13+
describe('Magento_Ui/js/form/element/date-time', function () {
14+
var injector = new Squire(),
15+
mocks = {
16+
'Magento_Ui/js/lib/registry/registry': {
17+
/** Method stub. */
18+
get: function () {
19+
return {
20+
get: jasmine.createSpy(),
21+
set: jasmine.createSpy()
22+
};
23+
},
24+
create: jasmine.createSpy(),
25+
set: jasmine.createSpy(),
26+
async: jasmine.createSpy()
27+
},
28+
'/mage/utils/wrapper': jasmine.createSpy()
29+
},
30+
model, utils, moment,
31+
dataScope = 'abstract';
32+
33+
beforeEach(function (done) {
34+
injector.mock(mocks);
35+
injector.require([
36+
'Magento_Ui/js/form/element/date',
37+
'mageUtils',
38+
'moment',
39+
'knockoutjs/knockout-es5'
40+
], function (Constr, mageUtils, m) {
41+
model = new Constr({
42+
provider: 'provName',
43+
name: '',
44+
index: '',
45+
dataScope: dataScope,
46+
options: {
47+
showsTime: true
48+
}
49+
});
50+
utils = mageUtils;
51+
moment = m;
52+
53+
done();
54+
});
2655
});
2756

2857
it('Check prepareDateTimeFormats function', function () {

0 commit comments

Comments
 (0)