Skip to content

Commit 546cde3

Browse files
author
Valeriy Nayda
committed
MAGETWO-43822: Incorrect Request Path generated for child category if Edit URL Rewrites from Category Page
- fix builds
1 parent a4056d4 commit 546cde3

File tree

8 files changed

+140
-55
lines changed

8 files changed

+140
-55
lines changed

‎app/code/Magento/CatalogUrlRewrite/Model/CategoryUrlPathGenerator.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ public function getUrlPath($category)
7474
}
7575
if ($this->isNeedToGenerateUrlPathForParent($category)) {
7676
$parentPath = $this->getUrlPath(
77-
$this->categoryRepository->get($category->getParentId(),$category->getStoreId())
77+
$this->categoryRepository->get($category->getParentId(), $category->getStoreId())
7878
);
7979
$path = $parentPath === '' ? $path : $parentPath . '/' . $path;
8080
}
@@ -143,7 +143,7 @@ public function getCanonicalUrlPath($category)
143143
* @param \Magento\Catalog\Model\Category $category
144144
* @return string
145145
*/
146-
public function generateUrlKey($category)
146+
public function getUrlKey($category)
147147
{
148148
$urlKey = $category->getUrlKey();
149149
return $category->formatUrlKey($urlKey === '' || $urlKey === null ? $category->getName() : $urlKey);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ public function getCanonicalUrlPath($product, $category = null)
114114
* @param \Magento\Catalog\Model\Product $product
115115
* @return string
116116
*/
117-
public function generateUrlKey($product)
117+
public function getUrlKey($product)
118118
{
119119
return $product->getUrlKey() === false ? false : $this->prepareProductUrlKey($product);
120120
}

‎app/code/Magento/CatalogUrlRewrite/Observer/CategoryUrlPathAutogeneratorObserver.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public function execute(\Magento\Framework\Event\Observer $observer)
4848
/** @var Category $category */
4949
$category = $observer->getEvent()->getCategory();
5050
if ($category->getUrlKey() !== false) {
51-
$category->setUrlKey($this->categoryUrlPathGenerator->generateUrlKey($category))
51+
$category->setUrlKey($this->categoryUrlPathGenerator->getUrlKey($category))
5252
->setUrlPath($this->categoryUrlPathGenerator->getUrlPath($category));
5353
if (!$category->isObjectNew()) {
5454
$category->getResource()->saveAttribute($category, 'url_path');
@@ -105,7 +105,6 @@ protected function isGlobalScope($storeId)
105105
*/
106106
protected function updateUrlPathForCategory(Category $category)
107107
{
108-
$category->unsUrlPath();
109108
$category->setUrlPath($this->categoryUrlPathGenerator->getUrlPath($category));
110109
$category->getResource()->saveAttribute($category, 'url_path');
111110
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,6 @@ public function execute(\Magento\Framework\Event\Observer $observer)
3131
{
3232
/** @var Product $product */
3333
$product = $observer->getEvent()->getProduct();
34-
$product->setUrlKey($this->productUrlPathGenerator->generateUrlKey($product));
34+
$product->setUrlKey($this->productUrlPathGenerator->getUrlKey($product));
3535
}
3636
}

‎app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/CategoryUrlPathGeneratorTest.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ public function testGetCanonicalUrlPath()
235235
/**
236236
* @return array
237237
*/
238-
public function generateUrlKeyDataProvider()
238+
public function getUrlKeyDataProvider()
239239
{
240240
return [
241241
['url-key', null, 'url-key'],
@@ -244,17 +244,17 @@ public function generateUrlKeyDataProvider()
244244
}
245245

246246
/**
247-
* @dataProvider generateUrlKeyDataProvider
248-
* @param string $urlKey
249-
* @param string $name
247+
* @dataProvider getUrlKeyDataProvider
248+
* @param string|null|bool $urlKey
249+
* @param string|null|bool $name
250250
* @param string $result
251251
*/
252-
public function testGenerateUrlKey($urlKey, $name, $result)
252+
public function testGetUrlKey($urlKey, $name, $result)
253253
{
254254
$this->category->expects($this->once())->method('getUrlKey')->will($this->returnValue($urlKey));
255255
$this->category->expects($this->any())->method('getName')->will($this->returnValue($name));
256256
$this->category->expects($this->once())->method('formatUrlKey')->will($this->returnArgument(0));
257257

258-
$this->assertEquals($result, $this->categoryUrlPathGenerator->generateUrlKey($this->category));
258+
$this->assertEquals($result, $this->categoryUrlPathGenerator->getUrlKey($this->category));
259259
}
260260
}

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

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -85,11 +85,11 @@ public function getUrlPathDataProvider()
8585

8686
/**
8787
* @dataProvider getUrlPathDataProvider
88-
* @param $urlKey
89-
* @param $productName
90-
* @param $result
88+
* @param string|null|bool $urlKey
89+
* @param string|null|bool $productName
90+
* @param string $result
9191
*/
92-
public function testGenerateUrlPath($urlKey, $productName, $result)
92+
public function testGetUrlPath($urlKey, $productName, $result)
9393
{
9494
$this->product->expects($this->once())->method('getData')->with('url_path')
9595
->will($this->returnValue(null));
@@ -101,37 +101,32 @@ public function testGenerateUrlPath($urlKey, $productName, $result)
101101
}
102102

103103
/**
104-
* @param $productUrlKey
105-
* @param $expectedUrlKey
106-
*
107-
* @dataProvider generateUrlKeyDataProvider
104+
* @param string|bool $productUrlKey
105+
* @param string|bool $expectedUrlKey
106+
* @dataProvider getUrlKeyDataProvider
108107
*/
109-
public function testGenerateUrlKey($productUrlKey, $expectedUrlKey)
108+
public function testGetUrlKey($productUrlKey, $expectedUrlKey)
110109
{
111110
$this->product->expects($this->any())->method('getUrlKey')->will($this->returnValue($productUrlKey));
112111
$this->product->expects($this->any())->method('formatUrlKey')->will($this->returnValue($productUrlKey));
113-
$this->assertEquals($expectedUrlKey, $this->productUrlPathGenerator->generateUrlKey($this->product));
112+
$this->assertEquals($expectedUrlKey, $this->productUrlPathGenerator->getUrlKey($this->product));
114113
}
115114

116-
public function generateUrlKeyDataProvider()
115+
/**
116+
* @return array
117+
*/
118+
public function getUrlKeyDataProvider()
117119
{
118120
return [
119121
'URL Key use default' => [false, false],
120122
'URL Key empty' => ['product-url', 'product-url'],
121123
];
122124
}
123125

124-
public function testGetUrlPath()
125-
{
126-
$this->product->expects($this->once())->method('getData')->with('url_path')
127-
->will($this->returnValue('url-path'));
128-
$this->product->expects($this->never())->method('getUrlKey');
129-
130-
$this->assertEquals('url-path', $this->productUrlPathGenerator->getUrlPath($this->product, null));
131-
}
132-
133126
/**
134-
*
127+
* @param string|null|bool $storedUrlKey
128+
* @param string|null|bool $productName
129+
* @param string $expectedUrlKey
135130
* @dataProvider getUrlPathDefaultUrlKeyDataProvider
136131
*/
137132
public function testGetUrlPathDefaultUrlKey($storedUrlKey, $productName, $expectedUrlKey)
@@ -144,13 +139,15 @@ public function testGetUrlPathDefaultUrlKey($storedUrlKey, $productName, $expect
144139
$this->assertEquals($expectedUrlKey, $this->productUrlPathGenerator->getUrlPath($this->product, null));
145140
}
146141

142+
/**
143+
* @return array
144+
*/
147145
public function getUrlPathDefaultUrlKeyDataProvider()
148146
{
149147
return [
150148
['default-store-view-url-key', null, 'default-store-view-url-key'],
151149
[false, 'default-store-view-product-name', 'default-store-view-product-name']
152150
];
153-
154151
}
155152

156153
public function testGetUrlPathWithCategory()

‎app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/CategoryUrlPathAutogeneratorObserverTest.php

Lines changed: 109 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,16 @@ class CategoryUrlPathAutogeneratorObserverTest extends \PHPUnit_Framework_TestCa
2424
/** @var \PHPUnit_Framework_MockObject_MockObject */
2525
protected $category;
2626

27+
/**
28+
* @var \Magento\CatalogUrlRewrite\Service\V1\StoreViewService|\PHPUnit_Framework_MockObject_MockObject
29+
*/
30+
protected $storeViewService;
31+
32+
/**
33+
* @var \Magento\Catalog\Model\ResourceModel\Category|\PHPUnit_Framework_MockObject_MockObject
34+
*/
35+
protected $categoryResource;
36+
2737
protected function setUp()
2838
{
2939
$this->observer = $this->getMock(
@@ -33,13 +43,15 @@ protected function setUp()
3343
'',
3444
false
3545
);
46+
$this->categoryResource = $this->getMock('Magento\Catalog\Model\ResourceModel\Category', [], [], '', false);
3647
$this->category = $this->getMock(
3748
'Magento\Catalog\Model\Category',
38-
['setUrlKey', 'setUrlPath', 'dataHasChangedFor', 'isObjectNew', 'getResource', 'getUrlKey'],
49+
['setUrlKey', 'setUrlPath', 'dataHasChangedFor', 'isObjectNew', 'getResource', 'getUrlKey', 'getStoreId'],
3950
[],
4051
'',
4152
false
4253
);
54+
$this->category->expects($this->any())->method('getResource')->willReturn($this->categoryResource);
4355
$this->observer->expects($this->any())->method('getEvent')->willReturnSelf();
4456
$this->observer->expects($this->any())->method('getCategory')->willReturn($this->category);
4557
$this->categoryUrlPathGenerator = $this->getMock(
@@ -53,19 +65,23 @@ protected function setUp()
5365
'Magento\CatalogUrlRewrite\Model\Category\ChildrenCategoriesProvider'
5466
);
5567

68+
$this->storeViewService = $this->getMock('Magento\CatalogUrlRewrite\Service\V1\StoreViewService', [], [], '',
69+
false);
70+
5671
$this->categoryUrlPathAutogeneratorObserver = (new ObjectManagerHelper($this))->getObject(
5772
'Magento\CatalogUrlRewrite\Observer\CategoryUrlPathAutogeneratorObserver',
5873
[
5974
'categoryUrlPathGenerator' => $this->categoryUrlPathGenerator,
60-
'childrenCategoriesProvider' => $this->childrenCategoriesProvider
75+
'childrenCategoriesProvider' => $this->childrenCategoriesProvider,
76+
'storeViewService' => $this->storeViewService,
6177
]
6278
);
6379
}
6480

6581
public function testSetCategoryUrlAndCategoryPath()
6682
{
6783
$this->category->expects($this->once())->method('getUrlKey')->willReturn('category');
68-
$this->categoryUrlPathGenerator->expects($this->once())->method('generateUrlKey')->willReturn('urk_key');
84+
$this->categoryUrlPathGenerator->expects($this->once())->method('getUrlKey')->willReturn('urk_key');
6985
$this->category->expects($this->once())->method('setUrlKey')->with('urk_key')->willReturnSelf();
7086
$this->categoryUrlPathGenerator->expects($this->once())->method('getUrlPath')->willReturn('url_path');
7187
$this->category->expects($this->once())->method('setUrlPath')->with('url_path')->willReturnSelf();
@@ -74,38 +90,111 @@ public function testSetCategoryUrlAndCategoryPath()
7490
$this->categoryUrlPathAutogeneratorObserver->execute($this->observer);
7591
}
7692

77-
public function testExecuteWithoutGeneration()
93+
public function testExecuteWithoutUrlKeyAndUrlPathUpdating()
7894
{
7995
$this->category->expects($this->once())->method('getUrlKey')->willReturn(false);
8096
$this->category->expects($this->never())->method('setUrlKey');
8197
$this->category->expects($this->never())->method('setUrlPath');
8298
$this->categoryUrlPathAutogeneratorObserver->execute($this->observer);
8399
}
84100

85-
public function testUpdateUrlPathForChildren()
101+
public function testUrlKeyAndUrlPathUpdating()
86102
{
87-
$this->category->expects($this->once())->method('getUrlKey')->willReturn('category');
88-
$this->category->expects($this->once())->method('setUrlKey')->willReturnSelf();
89-
$this->category->expects($this->once())->method('setUrlPath')->willReturnSelf();
103+
$this->categoryUrlPathGenerator->expects($this->once())->method('getUrlKey')->with($this->category)
104+
->willReturn('url_key');
105+
$this->categoryUrlPathGenerator->expects($this->once())->method('getUrlPath')->with($this->category)
106+
->willReturn('url_path');
107+
108+
$this->category->expects($this->once())->method('getUrlKey')->willReturn('not_formatted_url_key');
109+
$this->category->expects($this->once())->method('setUrlKey')->with('url_key')->willReturnSelf();
110+
$this->category->expects($this->once())->method('setUrlPath')->with('url_path')->willReturnSelf();
111+
// break code execution
112+
$this->category->expects($this->once())->method('isObjectNew')->willReturn(true);
113+
114+
$this->categoryUrlPathAutogeneratorObserver->execute($this->observer);
115+
}
116+
117+
public function testUrlPathAttributeNoUpdatingIfCategoryIsNew()
118+
{
119+
$this->categoryUrlPathGenerator->expects($this->any())->method('getUrlKey')->willReturn('url_key');
120+
$this->categoryUrlPathGenerator->expects($this->any())->method('getUrlPath')->willReturn('url_path');
121+
122+
$this->category->expects($this->any())->method('getUrlKey')->willReturn('not_formatted_url_key');
123+
$this->category->expects($this->any())->method('setUrlKey')->willReturnSelf();
124+
$this->category->expects($this->any())->method('setUrlPath')->willReturnSelf();
125+
126+
$this->category->expects($this->once())->method('isObjectNew')->willReturn(true);
127+
$this->categoryResource->expects($this->never())->method('saveAttribute');
128+
129+
$this->categoryUrlPathAutogeneratorObserver->execute($this->observer);
130+
}
131+
132+
public function testUrlPathAttributeUpdating()
133+
{
134+
$this->categoryUrlPathGenerator->expects($this->any())->method('getUrlKey')->willReturn('url_key');
135+
$this->categoryUrlPathGenerator->expects($this->any())->method('getUrlPath')->willReturn('url_path');
136+
137+
$this->category->expects($this->any())->method('getUrlKey')->willReturn('not_formatted_url_key');
138+
$this->category->expects($this->any())->method('setUrlKey')->willReturnSelf();
139+
$this->category->expects($this->any())->method('setUrlPath')->willReturnSelf();
90140
$this->category->expects($this->once())->method('isObjectNew')->willReturn(false);
91-
$this->category->expects($this->once())->method('dataHasChangedFor')->with('url_path')->willReturn(true);
92-
$categoryResource = $this->getMockBuilder('Magento\Catalog\Model\ResourceModel\Category')
93-
->disableOriginalConstructor()->getMock();
94-
$this->category->expects($this->once())->method('getResource')->willReturn($categoryResource);
95-
$categoryResource->expects($this->once())->method('saveAttribute')->with($this->category, 'url_path');
96141

142+
$this->categoryResource->expects($this->once())->method('saveAttribute')->with($this->category, 'url_path');
143+
144+
// break code execution
145+
$this->category->expects($this->once())->method('dataHasChangedFor')->with('url_path')->willReturn(false);
146+
147+
$this->categoryUrlPathAutogeneratorObserver->execute($this->observer);
148+
}
149+
150+
public function testChildrenUrlPathAttributeNoUpdatingIfParentUrlPathIsNotChanged()
151+
{
152+
$this->categoryUrlPathGenerator->expects($this->any())->method('getUrlKey')->willReturn('url_key');
153+
$this->categoryUrlPathGenerator->expects($this->any())->method('getUrlPath')->willReturn('url_path');
154+
155+
$this->categoryResource->expects($this->once())->method('saveAttribute')->with($this->category, 'url_path');
156+
157+
$this->category->expects($this->any())->method('getUrlKey')->willReturn('not_formatted_url_key');
158+
$this->category->expects($this->any())->method('setUrlKey')->willReturnSelf();
159+
$this->category->expects($this->any())->method('setUrlPath')->willReturnSelf();
160+
$this->category->expects($this->once())->method('isObjectNew')->willReturn(false);
161+
// break code execution
162+
$this->category->expects($this->once())->method('dataHasChangedFor')->with('url_path')->willReturn(false);
163+
164+
$this->categoryUrlPathAutogeneratorObserver->execute($this->observer);
165+
}
166+
167+
public function testChildrenUrlPathAttributeUpdatingForSpecificStore()
168+
{
169+
$this->categoryUrlPathGenerator->expects($this->any())->method('getUrlKey')->willReturn('generated_url_key');
170+
$this->categoryUrlPathGenerator->expects($this->any())->method('getUrlPath')->willReturn('generated_url_path');
171+
172+
$this->category->expects($this->any())->method('getUrlKey')->willReturn('not_formatted_url_key');
173+
$this->category->expects($this->any())->method('setUrlKey')->willReturnSelf();
174+
$this->category->expects($this->any())->method('setUrlPath')->willReturnSelf();
175+
$this->category->expects($this->any())->method('isObjectNew')->willReturn(false);
176+
$this->category->expects($this->any())->method('dataHasChangedFor')->willReturn(true);
177+
// only for specific store
178+
$this->category->expects($this->atLeastOnce())->method('getStoreId')->willReturn(1);
179+
180+
$childCategoryResource = $this->getMockBuilder('Magento\Catalog\Model\ResourceModel\Category')
181+
->disableOriginalConstructor()->getMock();
97182
$childCategory = $this->getMockBuilder('Magento\Catalog\Model\Category')
98-
->setMethods(['getUrlPath', 'setUrlPath', 'getResource', 'unsUrlPath'])
183+
->setMethods([
184+
'getUrlPath',
185+
'setUrlPath',
186+
'getResource',
187+
'getStore',
188+
'getStoreId',
189+
'setStoreId'
190+
])
99191
->disableOriginalConstructor()->getMock();
192+
$childCategory->expects($this->any())->method('getResource')->willReturn($childCategoryResource);
193+
$childCategory->expects($this->once())->method('setStoreId')->with(1);
100194

101195
$this->childrenCategoriesProvider->expects($this->once())->method('getChildren')->willReturn([$childCategory]);
102-
$childCategoryResource = $this->getMockBuilder('Magento\Catalog\Model\ResourceModel\Category')
103-
->disableOriginalConstructor()->getMock();
104-
$childCategory->expects($this->once())->method('unsUrlPath')->willReturnSelf();
105-
$childCategory->expects($this->once())->method('getResource')->willReturn($childCategoryResource);
196+
$childCategory->expects($this->once())->method('setUrlPath')->with('generated_url_path')->willReturnSelf();
106197
$childCategoryResource->expects($this->once())->method('saveAttribute')->with($childCategory, 'url_path');
107-
$childCategory->expects($this->once())->method('setUrlPath')->with('category-url_path')->willReturnSelf();
108-
$this->categoryUrlPathGenerator->expects($this->any())->method('getUrlPath')->willReturn('category-url_path');
109198

110199
$this->categoryUrlPathAutogeneratorObserver->execute($this->observer);
111200
}

‎app/code/Magento/ConfigurableProduct/Pricing/Price/ConfigurablePriceResolver.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public function resolvePrice(\Magento\Framework\Pricing\SaleableInterface $produ
5050
}
5151
if (!$price) {
5252
throw new \Magento\Framework\Exception\LocalizedException(
53-
__('Configurable product "%s" do not have sub-products', $product->getName())
53+
__('Configurable product "%1" do not have sub-products', $product->getName())
5454
);
5555
}
5656
return (float)$price;

0 commit comments

Comments
 (0)