Skip to content

Commit 3be13d1

Browse files
slopukhovslopukhov
committed
MAGETWO-70651: Saving category create not valid url-rewrites
1 parent 80fddbd commit 3be13d1

File tree

5 files changed

+194
-39
lines changed

5 files changed

+194
-39
lines changed

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

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ public function __construct(
3737
* @param \Magento\Catalog\Model\Category $category
3838
* @param int|null $rootCategoryId
3939
* @return \Magento\UrlRewrite\Service\V1\Data\UrlRewrite[]
40-
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
4140
*/
4241
public function generate(Product $product, Category $category, $rootCategoryId = null)
4342
{
@@ -47,19 +46,11 @@ public function generate(Product $product, Category $category, $rootCategoryId =
4746

4847
$storeId = $product->getStoreId();
4948

50-
$productCategories = $product->getCategoryCollection()
51-
->addAttributeToSelect('url_key')
52-
->addAttributeToSelect('url_path');
53-
5449
$urls = $this->productScopeRewriteGenerator->isGlobalScope($storeId)
55-
? $this->productScopeRewriteGenerator->generateForGlobalScope(
56-
$productCategories,
57-
$product,
58-
$rootCategoryId
59-
)
50+
? $this->productScopeRewriteGenerator->generateForGlobalScope([$category], $product, $rootCategoryId)
6051
: $this->productScopeRewriteGenerator->generateForSpecificStoreView(
6152
$storeId,
62-
$productCategories,
53+
[$category],
6354
$product,
6455
$rootCategoryId
6556
);

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

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
namespace Magento\CatalogUrlRewrite\Model;
7+
8+
use Magento\Catalog\Model\Product;
9+
use Magento\Catalog\Model\Product\Visibility;
10+
11+
/**
12+
* Class ProductUrlRewriteGenerator
13+
* @package Magento\CatalogUrlRewrite\Model
14+
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
15+
*/
16+
class CategoryProductUrlPathGenerator
17+
{
18+
/**
19+
* @var ProductScopeRewriteGenerator
20+
*/
21+
private $productScopeRewriteGenerator;
22+
23+
/**
24+
* @param ProductScopeRewriteGenerator $productScopeRewriteGenerator
25+
*/
26+
public function __construct(
27+
ProductScopeRewriteGenerator $productScopeRewriteGenerator
28+
) {
29+
$this->productScopeRewriteGenerator = $productScopeRewriteGenerator;
30+
}
31+
32+
/**
33+
* Generate product url rewrites based on category
34+
*
35+
* @param \Magento\Catalog\Model\Product $product
36+
* @param int|null $rootCategoryId
37+
* @return \Magento\UrlRewrite\Service\V1\Data\UrlRewrite[]
38+
*/
39+
public function generate(Product $product, $rootCategoryId = null)
40+
{
41+
if ($product->getVisibility() == Visibility::VISIBILITY_NOT_VISIBLE) {
42+
return [];
43+
}
44+
45+
$storeId = $product->getStoreId();
46+
47+
$productCategories = $product->getCategoryCollection()
48+
->addAttributeToSelect('url_key')
49+
->addAttributeToSelect('url_path');
50+
51+
$urls = $this->productScopeRewriteGenerator->isGlobalScope($storeId)
52+
? $this->productScopeRewriteGenerator->generateForGlobalScope(
53+
$productCategories,
54+
$product,
55+
$rootCategoryId
56+
)
57+
: $this->productScopeRewriteGenerator->generateForSpecificStoreView(
58+
$storeId,
59+
$productCategories,
60+
$product,
61+
$rootCategoryId
62+
);
63+
64+
return $urls;
65+
}
66+
}

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ public function generateProductUrlRewrites(\Magento\Catalog\Model\Category $cate
130130
* @param int|null $rootCategoryId
131131
* @return array
132132
*/
133-
public function getCategoryProductsUrlRewrites(
133+
private function getCategoryProductsUrlRewrites(
134134
\Magento\Catalog\Model\Category $category,
135135
$storeId,
136136
$saveRewriteHistory,
@@ -158,24 +158,24 @@ public function getCategoryProductsUrlRewrites(
158158
$product->setStoreId($storeId);
159159
$product->setData('save_rewrites_history', $saveRewriteHistory);
160160
$mergeDataProvider->merge(
161-
$this->getCategoryBasedProductRewriteGenerator()->generate($product, $category, $rootCategoryId)
161+
$this->getCategoryBasedProductRewriteGenerator()->generate($product, $rootCategoryId)
162162
);
163163
}
164164

165165
return $mergeDataProvider->getData();
166166
}
167167

168168
/**
169-
* Retrieve generator, which use single category for different products
169+
* Retrieve generator, which use all product categories for different products
170170
*
171171
* @deprecated
172-
* @return \Magento\CatalogUrlRewrite\Model\CategoryBasedProductRewriteGenerator|mixed
172+
* @return \Magento\CatalogUrlRewrite\Model\CategoryProductUrlPathGenerator|mixed
173173
*/
174174
private function getCategoryBasedProductRewriteGenerator()
175175
{
176176
if (!$this->categoryBasedProductRewriteGenerator) {
177177
$this->categoryBasedProductRewriteGenerator = \Magento\Framework\App\ObjectManager::getInstance()
178-
->get(\Magento\CatalogUrlRewrite\Model\CategoryBasedProductRewriteGenerator::class);
178+
->get(\Magento\CatalogUrlRewrite\Model\CategoryProductUrlPathGenerator::class);
179179
}
180180

181181
return $this->categoryBasedProductRewriteGenerator;

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

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
use Magento\Catalog\Model\Product;
1010
use Magento\CatalogUrlRewrite\Model\CategoryBasedProductRewriteGenerator;
1111
use Magento\CatalogUrlRewrite\Model\ProductScopeRewriteGenerator;
12-
use Magento\Catalog\Model\ResourceModel\Category\Collection;
1312

1413
/**
1514
* Class CategoryBasedProductRewriteGeneratorTest
@@ -42,9 +41,6 @@ public function testGenerationWithGlobalScope()
4241
$categoryMock = $this->getMockBuilder(Category::class)
4342
->disableOriginalConstructor()
4443
->getMock();
45-
$categoryCollectionMock = $this->getMockBuilder(Collection::class)
46-
->disableOriginalConstructor()
47-
->getMock();
4844
$productMock = $this->getMockBuilder(Product::class)
4945
->disableOriginalConstructor()
5046
->getMock();
@@ -58,20 +54,13 @@ public function testGenerationWithGlobalScope()
5854
$productMock->expects($this->once())
5955
->method('getStoreId')
6056
->willReturn($storeId);
61-
$productMock->expects($this->once())
62-
->method('getCategoryCollection')
63-
->willReturn($categoryCollectionMock);
64-
$categoryCollectionMock->expects($this->atLeastOnce())
65-
->method('addAttributeToSelect')
66-
->willReturnSelf();
67-
6857
$this->productScopeRewriteGeneratorMock->expects($this->once())
6958
->method('isGlobalScope')
7059
->with($storeId)
7160
->willReturn(true);
7261
$this->productScopeRewriteGeneratorMock->expects($this->once())
7362
->method('generateForGlobalScope')
74-
->with($categoryCollectionMock, $productMock, $categoryId)
63+
->with([$categoryMock], $productMock, $categoryId)
7564
->willReturn($urls);
7665

7766
$this->assertEquals($urls, $this->generator->generate($productMock, $categoryMock, $categoryId));
@@ -82,9 +71,6 @@ public function testGenerationWithSpecificStore()
8271
$categoryMock = $this->getMockBuilder(Category::class)
8372
->disableOriginalConstructor()
8473
->getMock();
85-
$categoryCollectionMock = $this->getMockBuilder(Collection::class)
86-
->disableOriginalConstructor()
87-
->getMock();
8874
$productMock = $this->getMockBuilder(Product::class)
8975
->disableOriginalConstructor()
9076
->getMock();
@@ -98,20 +84,13 @@ public function testGenerationWithSpecificStore()
9884
$productMock->expects($this->once())
9985
->method('getStoreId')
10086
->willReturn($storeId);
101-
$productMock->expects($this->once())
102-
->method('getCategoryCollection')
103-
->willReturn($categoryCollectionMock);
104-
$categoryCollectionMock->expects($this->atLeastOnce())
105-
->method('addAttributeToSelect')
106-
->willReturnSelf();
107-
10887
$this->productScopeRewriteGeneratorMock->expects($this->once())
10988
->method('isGlobalScope')
11089
->with($storeId)
11190
->willReturn(false);
11291
$this->productScopeRewriteGeneratorMock->expects($this->once())
11392
->method('generateForSpecificStoreView')
114-
->with($storeId, $categoryCollectionMock, $productMock, $categoryId)
93+
->with($storeId, [$categoryMock], $productMock, $categoryId)
11594
->willReturn($urls);
11695

11796
$this->assertEquals($urls, $this->generator->generate($productMock, $categoryMock, $categoryId));

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

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
namespace Magento\CatalogUrlRewrite\Test\Unit\Model;
7+
8+
use Magento\Catalog\Model\Category;
9+
use Magento\Catalog\Model\Product;
10+
use Magento\CatalogUrlRewrite\Model\CategoryProductUrlPathGenerator;
11+
use Magento\CatalogUrlRewrite\Model\ProductScopeRewriteGenerator;
12+
use Magento\Catalog\Model\ResourceModel\Category\Collection;
13+
14+
/**
15+
* Class CategoryProductUrlPathGeneratorTest
16+
*/
17+
class CategoryProductUrlPathGeneratorTest extends \PHPUnit_Framework_TestCase
18+
{
19+
/**
20+
* @var ProductScopeRewriteGenerator|\PHPUnit_Framework_MockObject_MockObject
21+
*/
22+
private $productScopeRewriteGeneratorMock;
23+
24+
/**
25+
* @var CategoryProductUrlPathGenerator
26+
*/
27+
private $generator;
28+
29+
public function setUp()
30+
{
31+
$this->productScopeRewriteGeneratorMock = $this->getMockBuilder(ProductScopeRewriteGenerator::class)
32+
->disableOriginalConstructor()
33+
->getMock();
34+
35+
$this->generator = new CategoryProductUrlPathGenerator(
36+
$this->productScopeRewriteGeneratorMock
37+
);
38+
}
39+
40+
public function testGenerationWithGlobalScope()
41+
{
42+
$categoryMock = $this->getMockBuilder(Category::class)
43+
->disableOriginalConstructor()
44+
->getMock();
45+
$categoryCollectionMock = $this->getMockBuilder(Collection::class)
46+
->disableOriginalConstructor()
47+
->getMock();
48+
$productMock = $this->getMockBuilder(Product::class)
49+
->disableOriginalConstructor()
50+
->getMock();
51+
$storeId = 1;
52+
$categoryId = 1;
53+
$urls = ['dummy-url.html'];
54+
55+
$productMock->expects($this->once())
56+
->method('getVisibility')
57+
->willReturn(2);
58+
$productMock->expects($this->once())
59+
->method('getStoreId')
60+
->willReturn($storeId);
61+
$productMock->expects($this->once())
62+
->method('getCategoryCollection')
63+
->willReturn($categoryCollectionMock);
64+
$categoryCollectionMock->expects($this->atLeastOnce())
65+
->method('addAttributeToSelect')
66+
->willReturnSelf();
67+
68+
$this->productScopeRewriteGeneratorMock->expects($this->once())
69+
->method('isGlobalScope')
70+
->with($storeId)
71+
->willReturn(true);
72+
$this->productScopeRewriteGeneratorMock->expects($this->once())
73+
->method('generateForGlobalScope')
74+
->with($categoryCollectionMock, $productMock, $categoryId)
75+
->willReturn($urls);
76+
77+
$this->assertEquals($urls, $this->generator->generate($productMock, $categoryId));
78+
}
79+
80+
public function testGenerationWithSpecificStore()
81+
{
82+
$categoryMock = $this->getMockBuilder(Category::class)
83+
->disableOriginalConstructor()
84+
->getMock();
85+
$categoryCollectionMock = $this->getMockBuilder(Collection::class)
86+
->disableOriginalConstructor()
87+
->getMock();
88+
$productMock = $this->getMockBuilder(Product::class)
89+
->disableOriginalConstructor()
90+
->getMock();
91+
$storeId = 1;
92+
$categoryId = 1;
93+
$urls = ['dummy-url.html'];
94+
95+
$productMock->expects($this->once())
96+
->method('getVisibility')
97+
->willReturn(2);
98+
$productMock->expects($this->once())
99+
->method('getStoreId')
100+
->willReturn($storeId);
101+
$productMock->expects($this->once())
102+
->method('getCategoryCollection')
103+
->willReturn($categoryCollectionMock);
104+
$categoryCollectionMock->expects($this->atLeastOnce())
105+
->method('addAttributeToSelect')
106+
->willReturnSelf();
107+
108+
$this->productScopeRewriteGeneratorMock->expects($this->once())
109+
->method('isGlobalScope')
110+
->with($storeId)
111+
->willReturn(false);
112+
$this->productScopeRewriteGeneratorMock->expects($this->once())
113+
->method('generateForSpecificStoreView')
114+
->with($storeId, $categoryCollectionMock, $productMock, $categoryId)
115+
->willReturn($urls);
116+
117+
$this->assertEquals($urls, $this->generator->generate($productMock, $categoryId));
118+
}
119+
}

0 commit comments

Comments
 (0)