From 550434e94fa3b8d5ab5ece8064fee3ab539bf4f0 Mon Sep 17 00:00:00 2001 From: Pavel Bystritsky Date: Fri, 27 Oct 2017 16:33:34 +0300 Subject: [PATCH 1/4] GITHUB-8970: Cannot assign products to categories not under tree root. --- .../Model/ProductScopeRewriteGenerator.php | 6 +- .../ProductScopeRewriteGeneratorTest.php | 73 +++++++++++++++---- .../_files/product_with_category.php | 2 +- 3 files changed, 63 insertions(+), 18 deletions(-) diff --git a/app/code/Magento/CatalogUrlRewrite/Model/ProductScopeRewriteGenerator.php b/app/code/Magento/CatalogUrlRewrite/Model/ProductScopeRewriteGenerator.php index 81bbb08b0f39e..61bad6d071d9b 100644 --- a/app/code/Magento/CatalogUrlRewrite/Model/ProductScopeRewriteGenerator.php +++ b/app/code/Magento/CatalogUrlRewrite/Model/ProductScopeRewriteGenerator.php @@ -207,7 +207,11 @@ public function generateForSpecificStoreView($storeId, $productCategories, Produ */ public function isCategoryProperForGenerating(Category $category, $storeId) { - if ($category->getParentId() != \Magento\Catalog\Model\Category::TREE_ROOT_ID) { + $parentId = $category->getParentId(); + if ( + $parentId != Category::ROOT_CATEGORY_ID + && $parentId != Category::TREE_ROOT_ID + ) { list(, $rootCategoryId) = $category->getParentIds(); return $rootCategoryId == $this->storeManager->getStore($storeId)->getRootCategoryId(); } diff --git a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/ProductScopeRewriteGeneratorTest.php b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/ProductScopeRewriteGeneratorTest.php index 48399d5ef612b..c24954042dc7f 100644 --- a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/ProductScopeRewriteGeneratorTest.php +++ b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/ProductScopeRewriteGeneratorTest.php @@ -47,6 +47,9 @@ class ProductScopeRewriteGeneratorTest extends \PHPUnit\Framework\TestCase /** @var \Magento\Framework\Serialize\Serializer\Json|\PHPUnit_Framework_MockObject_MockObject */ private $serializer; + /** @var \Magento\Catalog\Model\Category|\PHPUnit_Framework_MockObject_MockObject */ + private $categoryMock; + public function setUp() { $this->serializer = $this->createMock(\Magento\Framework\Serialize\Serializer\Json::class); @@ -83,6 +86,10 @@ function ($value) { $this->storeViewService = $this->getMockBuilder(\Magento\CatalogUrlRewrite\Service\V1\StoreViewService::class) ->disableOriginalConstructor()->getMock(); $this->storeManager = $this->createMock(StoreManagerInterface::class); + $storeRootCategoryId = 2; + $store = $this->getMockBuilder(\Magento\Store\Model\Store::class)->disableOriginalConstructor()->getMock(); + $store->expects($this->any())->method('getRootCategoryId')->will($this->returnValue($storeRootCategoryId)); + $this->storeManager->expects($this->any())->method('getStore')->will($this->returnValue($store)); $mergeDataProviderFactory = $this->createPartialMock( \Magento\UrlRewrite\Model\MergeDataProviderFactory::class, ['create'] @@ -103,6 +110,7 @@ function ($value) { 'mergeDataProviderFactory' => $mergeDataProviderFactory ] ); + $this->categoryMock = $this->getMockBuilder(Category::class)->disableOriginalConstructor()->getMock(); } public function testGenerationForGlobalScope() @@ -112,12 +120,9 @@ public function testGenerationForGlobalScope() $product->expects($this->any())->method('getStoreIds')->will($this->returnValue([1])); $this->storeViewService->expects($this->once())->method('doesEntityHaveOverriddenUrlKeyForStore') ->will($this->returnValue(false)); - $categoryMock = $this->getMockBuilder(Category::class) - ->disableOriginalConstructor() - ->getMock(); - $categoryMock->expects($this->once()) + $this->categoryMock->expects($this->exactly(2)) ->method('getParentId') - ->willReturn(1); + ->willReturn(2); $this->initObjectRegistryFactory([]); $canonical = new \Magento\UrlRewrite\Service\V1\Data\UrlRewrite([], $this->serializer); $canonical->setRequestPath('category-1') @@ -149,25 +154,23 @@ public function testGenerationForGlobalScope() 'category-3_3' => $current, 'category-4_4' => $anchorCategories ], - $this->productScopeGenerator->generateForGlobalScope([$categoryMock], $product, 1) + $this->productScopeGenerator->generateForGlobalScope([$this->categoryMock], $product, 1) ); } public function testGenerationForSpecificStore() { + $storeRootCategoryId = 2; + $parent_id = 3; + $category_id = 4; $product = $this->createMock(\Magento\Catalog\Model\Product::class); $product->expects($this->any())->method('getStoreId')->will($this->returnValue(1)); $product->expects($this->never())->method('getStoreIds'); - $storeRootCategoryId = 'root-for-store-id'; - $category = $this->createMock(\Magento\Catalog\Model\Category::class); - $category->expects($this->any())->method('getParentIds') + $this->categoryMock->expects($this->any())->method('getParentIds') ->will($this->returnValue(['root-id', $storeRootCategoryId])); - $category->expects($this->any())->method('getParentId')->will($this->returnValue('parent_id')); - $category->expects($this->any())->method('getId')->will($this->returnValue('category_id')); - $store = $this->getMockBuilder(\Magento\Store\Model\Store::class)->disableOriginalConstructor()->getMock(); - $store->expects($this->any())->method('getRootCategoryId')->will($this->returnValue($storeRootCategoryId)); - $this->storeManager->expects($this->any())->method('getStore')->will($this->returnValue($store)); - $this->initObjectRegistryFactory([$category]); + $this->categoryMock->expects($this->any())->method('getParentId')->will($this->returnValue($parent_id)); + $this->categoryMock->expects($this->any())->method('getId')->will($this->returnValue($category_id)); + $this->initObjectRegistryFactory([$this->categoryMock]); $canonical = new \Magento\UrlRewrite\Service\V1\Data\UrlRewrite([], $this->serializer); $canonical->setRequestPath('category-1') ->setStoreId(1); @@ -184,7 +187,7 @@ public function testGenerationForSpecificStore() $this->assertEquals( ['category-1_1' => $canonical], - $this->productScopeGenerator->generateForSpecificStoreView(1, [$category], $product, 1) + $this->productScopeGenerator->generateForSpecificStoreView(1, [$this->categoryMock], $product, 1) ); } @@ -212,4 +215,42 @@ protected function initObjectRegistryFactory($entities) ->with(['entities' => $entities]) ->will($this->returnValue($objectRegistry)); } + + /** + * Test the possibility of url rewrite generation. + * + * @param int $parentId + * @param array $parentIds + * @param bool $expectedResult + * @dataProvider isCategoryProperForGeneratingDataProvider + */ + public function testIsCategoryProperForGenerating($parentId, $parentIds, $expectedResult) + { + $storeId = 1; + $this->categoryMock->expects(self::any())->method('getParentId')->willReturn($parentId); + $this->categoryMock->expects(self::any())->method('getParentIds')->willReturn($parentIds); + $result = $this->productScopeGenerator->isCategoryProperForGenerating( + $this->categoryMock, + $storeId + ); + self::assertEquals( + $expectedResult, + $result + ); + } + + /** + * Data provider for testIsCategoryProperForGenerating. + * + * @return array + */ + public function isCategoryProperForGeneratingDataProvider() + { + return [ + ['0', ['0'], false], + ['1', ['1'], false], + ['2', ['1', '2'], true], + ['2', ['1', '3'], false], + ]; + } } diff --git a/dev/tests/integration/testsuite/Magento/CatalogUrlRewrite/_files/product_with_category.php b/dev/tests/integration/testsuite/Magento/CatalogUrlRewrite/_files/product_with_category.php index 2e2be36743b0f..236a80a3e66f9 100644 --- a/dev/tests/integration/testsuite/Magento/CatalogUrlRewrite/_files/product_with_category.php +++ b/dev/tests/integration/testsuite/Magento/CatalogUrlRewrite/_files/product_with_category.php @@ -69,4 +69,4 @@ /** @var CategoryLinkManagementInterface $linkManagement */ $linkManagement = $objectManager->get(CategoryLinkManagementInterface::class); -$linkManagement->assignProductToCategories($product->getSku(), [$category->getEntityId()]); +$linkManagement->assignProductToCategories($product->getSku(), [Category::TREE_ROOT_ID, $category->getEntityId()]); From 566cc7fef2ab33d2b2ea296b2e4e8a83abf0838e Mon Sep 17 00:00:00 2001 From: Pavel Bystritsky Date: Mon, 30 Oct 2017 10:00:06 +0200 Subject: [PATCH 2/4] GITHUB-8970: Cannot assign products to categories not under tree root. --- .../Test/Unit/Model/ProductScopeRewriteGeneratorTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/ProductScopeRewriteGeneratorTest.php b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/ProductScopeRewriteGeneratorTest.php index c24954042dc7f..43a637458ddaa 100644 --- a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/ProductScopeRewriteGeneratorTest.php +++ b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/ProductScopeRewriteGeneratorTest.php @@ -120,7 +120,7 @@ public function testGenerationForGlobalScope() $product->expects($this->any())->method('getStoreIds')->will($this->returnValue([1])); $this->storeViewService->expects($this->once())->method('doesEntityHaveOverriddenUrlKeyForStore') ->will($this->returnValue(false)); - $this->categoryMock->expects($this->exactly(2)) + $this->categoryMock->expects($this->once()) ->method('getParentId') ->willReturn(2); $this->initObjectRegistryFactory([]); From f0be8db3a5e35c8ab69748e0990790fc0fb146a0 Mon Sep 17 00:00:00 2001 From: Pavel Bystritsky Date: Mon, 30 Oct 2017 10:15:56 +0200 Subject: [PATCH 3/4] GITHUB-8970: Cannot assign products to categories not under tree root. --- .../CatalogUrlRewrite/Model/ProductScopeRewriteGenerator.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/app/code/Magento/CatalogUrlRewrite/Model/ProductScopeRewriteGenerator.php b/app/code/Magento/CatalogUrlRewrite/Model/ProductScopeRewriteGenerator.php index 61bad6d071d9b..cf4ea3bc90ccc 100644 --- a/app/code/Magento/CatalogUrlRewrite/Model/ProductScopeRewriteGenerator.php +++ b/app/code/Magento/CatalogUrlRewrite/Model/ProductScopeRewriteGenerator.php @@ -208,10 +208,7 @@ public function generateForSpecificStoreView($storeId, $productCategories, Produ public function isCategoryProperForGenerating(Category $category, $storeId) { $parentId = $category->getParentId(); - if ( - $parentId != Category::ROOT_CATEGORY_ID - && $parentId != Category::TREE_ROOT_ID - ) { + if ($parentId != Category::ROOT_CATEGORY_ID && $parentId != Category::TREE_ROOT_ID) { list(, $rootCategoryId) = $category->getParentIds(); return $rootCategoryId == $this->storeManager->getStore($storeId)->getRootCategoryId(); } From de5990a2ef32135c2b774806373c45f5e5c43550 Mon Sep 17 00:00:00 2001 From: Pavel Bystritsky Date: Mon, 30 Oct 2017 11:35:22 +0200 Subject: [PATCH 4/4] GITHUB-8970: Cannot assign products to categories not under tree root. --- .../Model/ProductScopeRewriteGenerator.php | 6 +++--- .../Model/ProductScopeRewriteGeneratorTest.php | 17 +++++------------ 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/app/code/Magento/CatalogUrlRewrite/Model/ProductScopeRewriteGenerator.php b/app/code/Magento/CatalogUrlRewrite/Model/ProductScopeRewriteGenerator.php index cf4ea3bc90ccc..9c5c37b51f0b2 100644 --- a/app/code/Magento/CatalogUrlRewrite/Model/ProductScopeRewriteGenerator.php +++ b/app/code/Magento/CatalogUrlRewrite/Model/ProductScopeRewriteGenerator.php @@ -207,9 +207,9 @@ public function generateForSpecificStoreView($storeId, $productCategories, Produ */ public function isCategoryProperForGenerating(Category $category, $storeId) { - $parentId = $category->getParentId(); - if ($parentId != Category::ROOT_CATEGORY_ID && $parentId != Category::TREE_ROOT_ID) { - list(, $rootCategoryId) = $category->getParentIds(); + $parentIds = $category->getParentIds(); + if (count($parentIds) >= 2) { + $rootCategoryId = $parentIds[1]; return $rootCategoryId == $this->storeManager->getStore($storeId)->getRootCategoryId(); } return false; diff --git a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/ProductScopeRewriteGeneratorTest.php b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/ProductScopeRewriteGeneratorTest.php index 43a637458ddaa..06be01445df4c 100644 --- a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/ProductScopeRewriteGeneratorTest.php +++ b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/ProductScopeRewriteGeneratorTest.php @@ -120,9 +120,6 @@ public function testGenerationForGlobalScope() $product->expects($this->any())->method('getStoreIds')->will($this->returnValue([1])); $this->storeViewService->expects($this->once())->method('doesEntityHaveOverriddenUrlKeyForStore') ->will($this->returnValue(false)); - $this->categoryMock->expects($this->once()) - ->method('getParentId') - ->willReturn(2); $this->initObjectRegistryFactory([]); $canonical = new \Magento\UrlRewrite\Service\V1\Data\UrlRewrite([], $this->serializer); $canonical->setRequestPath('category-1') @@ -161,14 +158,12 @@ public function testGenerationForGlobalScope() public function testGenerationForSpecificStore() { $storeRootCategoryId = 2; - $parent_id = 3; $category_id = 4; $product = $this->createMock(\Magento\Catalog\Model\Product::class); $product->expects($this->any())->method('getStoreId')->will($this->returnValue(1)); $product->expects($this->never())->method('getStoreIds'); $this->categoryMock->expects($this->any())->method('getParentIds') ->will($this->returnValue(['root-id', $storeRootCategoryId])); - $this->categoryMock->expects($this->any())->method('getParentId')->will($this->returnValue($parent_id)); $this->categoryMock->expects($this->any())->method('getId')->will($this->returnValue($category_id)); $this->initObjectRegistryFactory([$this->categoryMock]); $canonical = new \Magento\UrlRewrite\Service\V1\Data\UrlRewrite([], $this->serializer); @@ -219,15 +214,13 @@ protected function initObjectRegistryFactory($entities) /** * Test the possibility of url rewrite generation. * - * @param int $parentId * @param array $parentIds * @param bool $expectedResult * @dataProvider isCategoryProperForGeneratingDataProvider */ - public function testIsCategoryProperForGenerating($parentId, $parentIds, $expectedResult) + public function testIsCategoryProperForGenerating($parentIds, $expectedResult) { $storeId = 1; - $this->categoryMock->expects(self::any())->method('getParentId')->willReturn($parentId); $this->categoryMock->expects(self::any())->method('getParentIds')->willReturn($parentIds); $result = $this->productScopeGenerator->isCategoryProperForGenerating( $this->categoryMock, @@ -247,10 +240,10 @@ public function testIsCategoryProperForGenerating($parentId, $parentIds, $expect public function isCategoryProperForGeneratingDataProvider() { return [ - ['0', ['0'], false], - ['1', ['1'], false], - ['2', ['1', '2'], true], - ['2', ['1', '3'], false], + [['0'], false], + [['1'], false], + [['1', '2'], true], + [['1', '3'], false], ]; } }