Skip to content

Commit 9a5c8ff

Browse files
arnobsharnobsh
committed
MC-20229: Different number of URL rewrites when editing or importing a product
- Anchors links on URL Rewrite for categories
1 parent 3490d3b commit 9a5c8ff

File tree

5 files changed

+206
-83
lines changed

5 files changed

+206
-83
lines changed

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

Lines changed: 59 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
use Magento\Framework\Event\Observer;
2424
use Magento\Framework\Event\ObserverInterface;
2525
use Magento\Framework\Exception\LocalizedException;
26-
use Magento\Framework\Exception\NoSuchEntityException;
2726
use Magento\ImportExport\Model\Import as ImportExport;
2827
use Magento\Store\Model\Store;
2928
use Magento\Store\Model\StoreManagerInterface;
@@ -252,7 +251,7 @@ public function execute(Observer $observer)
252251
* @throws LocalizedException
253252
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
254253
*/
255-
protected function _populateForUrlGeneration($rowData)
254+
private function _populateForUrlGeneration($rowData)
256255
{
257256
$newSku = $this->import->getNewSku($rowData[ImportProduct::COL_SKU]);
258257
$oldSku = $this->import->getOldSku();
@@ -321,7 +320,7 @@ private function isNeedToPopulateForUrlGeneration($rowData, $newSku, $oldSku): b
321320
* @param array $rowData
322321
* @return void
323322
*/
324-
protected function setStoreToProduct(Product $product, array $rowData)
323+
private function setStoreToProduct(Product $product, array $rowData)
325324
{
326325
if (!empty($rowData[ImportProduct::COL_STORE])
327326
&& ($storeId = $this->import->getStoreIdByCode($rowData[ImportProduct::COL_STORE]))
@@ -339,7 +338,7 @@ protected function setStoreToProduct(Product $product, array $rowData)
339338
* @param string $storeId
340339
* @return $this
341340
*/
342-
protected function addProductToImport($product, $storeId)
341+
private function addProductToImport($product, $storeId)
343342
{
344343
if ($product->getVisibility() == (string)Visibility::getOptionArray()[Visibility::VISIBILITY_NOT_VISIBLE]) {
345344
return $this;
@@ -357,7 +356,7 @@ protected function addProductToImport($product, $storeId)
357356
* @param Product $product
358357
* @return $this
359358
*/
360-
protected function populateGlobalProduct($product)
359+
private function populateGlobalProduct($product)
361360
{
362361
foreach ($this->import->getProductWebsites($product->getSku()) as $websiteId) {
363362
foreach ($this->websitesToStoreIds[$websiteId] as $storeId) {
@@ -376,7 +375,7 @@ protected function populateGlobalProduct($product)
376375
* @return UrlRewrite[]
377376
* @throws LocalizedException
378377
*/
379-
protected function generateUrls()
378+
private function generateUrls()
380379
{
381380
$mergeDataProvider = clone $this->mergeDataProviderPrototype;
382381
$mergeDataProvider->merge($this->canonicalUrlRewriteGenerate());
@@ -398,7 +397,7 @@ protected function generateUrls()
398397
* @param int|null $storeId
399398
* @return bool
400399
*/
401-
protected function isGlobalScope($storeId)
400+
private function isGlobalScope($storeId)
402401
{
403402
return null === $storeId || $storeId == Store::DEFAULT_STORE_ID;
404403
}
@@ -408,7 +407,7 @@ protected function isGlobalScope($storeId)
408407
*
409408
* @return UrlRewrite[]
410409
*/
411-
protected function canonicalUrlRewriteGenerate()
410+
private function canonicalUrlRewriteGenerate()
412411
{
413412
$urls = [];
414413
foreach ($this->products as $productId => $productsByStores) {
@@ -433,7 +432,7 @@ protected function canonicalUrlRewriteGenerate()
433432
* @return UrlRewrite[]
434433
* @throws LocalizedException
435434
*/
436-
protected function categoriesUrlRewriteGenerate()
435+
private function categoriesUrlRewriteGenerate(): array
437436
{
438437
$urls = [];
439438
foreach ($this->products as $productId => $productsByStores) {
@@ -444,25 +443,32 @@ protected function categoriesUrlRewriteGenerate()
444443
continue;
445444
}
446445
$requestPath = $this->productUrlPathGenerator->getUrlPathWithSuffix($product, $storeId, $category);
447-
$urls[] = $this->urlRewriteFactory->create()
448-
->setEntityType(ProductUrlRewriteGenerator::ENTITY_TYPE)
449-
->setEntityId($productId)
450-
->setRequestPath($requestPath)
451-
->setTargetPath($this->productUrlPathGenerator->getCanonicalUrlPath($product, $category))
452-
->setStoreId($storeId)
453-
->setMetadata(['category_id' => $category->getId()]);
446+
$urls[] = [
447+
$this->urlRewriteFactory->create()
448+
->setEntityType(ProductUrlRewriteGenerator::ENTITY_TYPE)
449+
->setEntityId($productId)
450+
->setRequestPath($requestPath)
451+
->setTargetPath($this->productUrlPathGenerator->getCanonicalUrlPath($product, $category))
452+
->setStoreId($storeId)
453+
->setMetadata(['category_id' => $category->getId()])
454+
];
455+
$parentCategoryIds = $category->getAnchorsAbove();
456+
if ($parentCategoryIds) {
457+
$urls[] = $this->getParentCategoriesUrlRewrites($parentCategoryIds, $storeId, $product);
458+
}
454459
}
455460
}
456461
}
457-
return $urls;
462+
$result = !empty($urls) ? array_merge(...$urls) : [];
463+
return $result;
458464
}
459465

460466
/**
461467
* Generate list based on current rewrites
462468
*
463469
* @return UrlRewrite[]
464470
*/
465-
protected function currentUrlRewritesRegenerate()
471+
private function currentUrlRewritesRegenerate()
466472
{
467473
$currentUrlRewrites = $this->urlFinder->findAllByData(
468474
[
@@ -496,7 +502,7 @@ protected function currentUrlRewritesRegenerate()
496502
* @param Category $category
497503
* @return array
498504
*/
499-
protected function generateForAutogenerated($url, $category)
505+
private function generateForAutogenerated($url, $category)
500506
{
501507
$storeId = $url->getStoreId();
502508
$productId = $url->getEntityId();
@@ -532,7 +538,7 @@ protected function generateForAutogenerated($url, $category)
532538
* @param Category $category
533539
* @return array
534540
*/
535-
protected function generateForCustom($url, $category)
541+
private function generateForCustom($url, $category)
536542
{
537543
$storeId = $url->getStoreId();
538544
$productId = $url->getEntityId();
@@ -566,7 +572,7 @@ protected function generateForCustom($url, $category)
566572
* @param UrlRewrite $url
567573
* @return Category|null|bool
568574
*/
569-
protected function retrieveCategoryFromMetadata($url)
575+
private function retrieveCategoryFromMetadata($url)
570576
{
571577
$metadata = $url->getMetadata();
572578
if (isset($metadata['category_id'])) {
@@ -576,32 +582,6 @@ protected function retrieveCategoryFromMetadata($url)
576582
return null;
577583
}
578584

579-
/**
580-
* Check, category suited for url-rewrite generation.
581-
*
582-
* @param Category $category
583-
* @param int $storeId
584-
* @return bool
585-
* @throws NoSuchEntityException
586-
*/
587-
protected function isCategoryProperForGenerating($category, $storeId)
588-
{
589-
if (isset($this->acceptableCategories[$storeId]) &&
590-
isset($this->acceptableCategories[$storeId][$category->getId()])) {
591-
return $this->acceptableCategories[$storeId][$category->getId()];
592-
}
593-
$acceptable = false;
594-
if ($category->getParentId() != Category::TREE_ROOT_ID) {
595-
list(, $rootCategoryId) = $category->getParentIds();
596-
$acceptable = ($rootCategoryId == $this->storeManager->getStore($storeId)->getRootCategoryId());
597-
}
598-
if (!isset($this->acceptableCategories[$storeId])) {
599-
$this->acceptableCategories[$storeId] = [];
600-
}
601-
$this->acceptableCategories[$storeId][$category->getId()] = $acceptable;
602-
return $acceptable;
603-
}
604-
605585
/**
606586
* Get category by id considering store scope.
607587
*
@@ -635,4 +615,36 @@ private function isCategoryRewritesEnabled()
635615
{
636616
return (bool)$this->scopeConfig->getValue('catalog/seo/generate_category_product_rewrites');
637617
}
618+
619+
/**
620+
* Generate url-rewrite for anchor parent-categories.
621+
*
622+
* @param array $categoryIds
623+
* @param int $storeId
624+
* @param Product $product
625+
* @return array
626+
* @throws LocalizedException
627+
*/
628+
private function getParentCategoriesUrlRewrites(array $categoryIds, int $storeId, Product $product): array
629+
{
630+
$urls = [];
631+
foreach ($categoryIds as $categoryId) {
632+
$category = $this->getCategoryById($categoryId, $storeId);
633+
if ($category->getParentId() == Category::TREE_ROOT_ID) {
634+
continue;
635+
}
636+
$requestPath = $this->productUrlPathGenerator
637+
->getUrlPathWithSuffix($product, $storeId, $category);
638+
$targetPath = $this->productUrlPathGenerator
639+
->getCanonicalUrlPath($product, $category);
640+
$urls[] = $this->urlRewriteFactory->create()
641+
->setEntityType(ProductUrlRewriteGenerator::ENTITY_TYPE)
642+
->setEntityId($product->getId())
643+
->setRequestPath($requestPath)
644+
->setTargetPath($targetPath)
645+
->setStoreId($storeId)
646+
->setMetadata(['category_id' => $category->getId()]);
647+
}
648+
return $urls;
649+
}
638650
}

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

Lines changed: 46 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -153,24 +153,32 @@ class AfterImportDataObserverTest extends \PHPUnit\Framework\TestCase
153153
*/
154154
protected function setUp()
155155
{
156-
$this->importProduct = $this->createPartialMock(\Magento\CatalogImportExport\Model\Import\Product::class, [
156+
$this->importProduct = $this->createPartialMock(
157+
\Magento\CatalogImportExport\Model\Import\Product::class,
158+
[
157159
'getNewSku',
158160
'getProductCategories',
159161
'getProductWebsites',
160162
'getStoreIdByCode',
161163
'getCategoryProcessor',
162-
]);
163-
$this->catalogProductFactory = $this->createPartialMock(\Magento\Catalog\Model\ProductFactory::class, [
164+
]
165+
);
166+
$this->catalogProductFactory = $this->createPartialMock(
167+
\Magento\Catalog\Model\ProductFactory::class,
168+
[
164169
'create',
165-
]);
170+
]
171+
);
166172
$this->storeManager = $this
167173
->getMockBuilder(
168174
\Magento\Store\Model\StoreManagerInterface::class
169175
)
170176
->disableOriginalConstructor()
171-
->setMethods([
172-
'getWebsite',
173-
])
177+
->setMethods(
178+
[
179+
'getWebsite',
180+
]
181+
)
174182
->getMockForAbstractClass();
175183
$this->event = $this->createPartialMock(\Magento\Framework\Event::class, ['getAdapter', 'getBunch']);
176184
$this->event->expects($this->any())->method('getAdapter')->willReturn($this->importProduct);
@@ -202,9 +210,11 @@ protected function setUp()
202210
);
203211
$this->urlFinder = $this
204212
->getMockBuilder(\Magento\UrlRewrite\Model\UrlFinderInterface::class)
205-
->setMethods([
206-
'findAllByData',
207-
])
213+
->setMethods(
214+
[
215+
'findAllByData',
216+
]
217+
)
208218
->disableOriginalConstructor()
209219
->getMockForAbstractClass();
210220

@@ -269,9 +279,12 @@ public function testAfterImportData()
269279
$newSku = [['entity_id' => 'value'], ['entity_id' => 'value3']];
270280
$websiteId = 'websiteId value';
271281
$productsCount = count($this->products);
272-
$websiteMock = $this->createPartialMock(\Magento\Store\Model\Website::class, [
282+
$websiteMock = $this->createPartialMock(
283+
\Magento\Store\Model\Website::class,
284+
[
273285
'getStoreIds',
274-
]);
286+
]
287+
);
275288
$storeIds = [1, Store::DEFAULT_STORE_ID];
276289
$websiteMock
277290
->expects($this->once())
@@ -315,13 +328,16 @@ public function testAfterImportData()
315328
->expects($this->exactly(1))
316329
->method('getStoreIdByCode')
317330
->will($this->returnValueMap($map));
318-
$product = $this->createPartialMock(\Magento\Catalog\Model\Product::class, [
331+
$product = $this->createPartialMock(
332+
\Magento\Catalog\Model\Product::class,
333+
[
319334
'getId',
320335
'setId',
321336
'getSku',
322337
'setStoreId',
323338
'getStoreId',
324-
]);
339+
]
340+
);
325341
$product
326342
->expects($this->exactly($productsCount))
327343
->method('setId')
@@ -341,17 +357,21 @@ public function testAfterImportData()
341357
$product
342358
->expects($this->exactly($productsCount))
343359
->method('getSku')
344-
->will($this->onConsecutiveCalls(
345-
$this->products[0]['sku'],
346-
$this->products[1]['sku']
347-
));
360+
->will(
361+
$this->onConsecutiveCalls(
362+
$this->products[0]['sku'],
363+
$this->products[1]['sku']
364+
)
365+
);
348366
$product
349367
->expects($this->exactly($productsCount))
350368
->method('getStoreId')
351-
->will($this->onConsecutiveCalls(
352-
$this->products[0][ImportProduct::COL_STORE],
353-
$this->products[1][ImportProduct::COL_STORE]
354-
));
369+
->will(
370+
$this->onConsecutiveCalls(
371+
$this->products[0][ImportProduct::COL_STORE],
372+
$this->products[1][ImportProduct::COL_STORE]
373+
)
374+
);
355375
$product
356376
->expects($this->exactly($productsCount))
357377
->method('setStoreId')
@@ -540,7 +560,10 @@ public function testCategoriesUrlRewriteGenerate()
540560
->expects($this->any())
541561
->method('getId')
542562
->will($this->returnValue($this->categoryId));
543-
563+
$category
564+
->expects($this->any())
565+
->method('getAnchorsAbove')
566+
->willReturn([]);
544567
$categoryCollection = $this->getMockBuilder(CategoryCollection::class)
545568
->disableOriginalConstructor()
546569
->getMock();

0 commit comments

Comments
 (0)