-
Notifications
You must be signed in to change notification settings - Fork 9.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
20434 consider url rewrite when change product visibility attribute 2 3 #20774
Changes from 9 commits
24382e3
4d18c9a
6a5776c
f2088d9
4edec38
a8d1dc0
5687704
c865bdc
1c608d7
a80943f
a8d9be1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,126 @@ | ||
<?php | ||
/** | ||
* Copyright © Magento, Inc. All rights reserved. | ||
* See COPYING.txt for license details. | ||
*/ | ||
declare(strict_types=1); | ||
|
||
namespace Magento\CatalogUrlRewrite\Model\Products; | ||
|
||
use Magento\Catalog\Api\Data\ProductInterface; | ||
use Magento\Catalog\Model\Product; | ||
use Magento\Catalog\Model\Product\Visibility; | ||
use Magento\Catalog\Model\ResourceModel\Product\CollectionFactory; | ||
use Magento\CatalogUrlRewrite\Model\ProductUrlPathGenerator; | ||
use Magento\CatalogUrlRewrite\Model\ProductUrlRewriteGenerator; | ||
use Magento\UrlRewrite\Model\Exception\UrlAlreadyExistsException; | ||
use Magento\UrlRewrite\Model\UrlPersistInterface; | ||
use Magento\UrlRewrite\Service\V1\Data\UrlRewrite; | ||
|
||
/** | ||
* Save/Delete UrlRewrites by Product ID's and visibility | ||
*/ | ||
class AdaptUrlRewritesToVisibilityAttribute | ||
{ | ||
/** | ||
* @var CollectionFactory | ||
*/ | ||
private $productCollectionFactory; | ||
|
||
/** | ||
* @var ProductUrlRewriteGenerator | ||
*/ | ||
private $urlRewriteGenerator; | ||
|
||
/** | ||
* @var UrlPersistInterface | ||
*/ | ||
private $urlPersist; | ||
|
||
/** | ||
* @var ProductUrlPathGenerator | ||
*/ | ||
private $urlPathGenerator; | ||
|
||
/** | ||
* @param CollectionFactory $collectionFactory | ||
* @param ProductUrlRewriteGenerator $urlRewriteGenerator | ||
* @param UrlPersistInterface $urlPersist | ||
* @param ProductUrlPathGenerator|null $urlPathGenerator | ||
*/ | ||
public function __construct( | ||
CollectionFactory $collectionFactory, | ||
ProductUrlRewriteGenerator $urlRewriteGenerator, | ||
UrlPersistInterface $urlPersist, | ||
ProductUrlPathGenerator $urlPathGenerator | ||
) { | ||
$this->productCollectionFactory = $collectionFactory; | ||
$this->urlRewriteGenerator = $urlRewriteGenerator; | ||
$this->urlPersist = $urlPersist; | ||
$this->urlPathGenerator = $urlPathGenerator; | ||
} | ||
|
||
/** | ||
* Process Url Rewrites according to the products visibility attribute | ||
* | ||
* @param array $productIds | ||
* @param int $visibility | ||
* @throws UrlAlreadyExistsException | ||
*/ | ||
public function execute(array $productIds, int $visibility): void | ||
{ | ||
$products = $this->getProductsByIds($productIds); | ||
|
||
/** @var Product $product */ | ||
foreach ($products as $product) { | ||
if ($visibility == Visibility::VISIBILITY_NOT_VISIBLE) { | ||
$this->urlPersist->deleteByData( | ||
[ | ||
UrlRewrite::ENTITY_ID => $product->getId(), | ||
UrlRewrite::ENTITY_TYPE => ProductUrlRewriteGenerator::ENTITY_TYPE, | ||
] | ||
); | ||
} elseif ($visibility !== Visibility::VISIBILITY_NOT_VISIBLE) { | ||
$product->setVisibility($visibility); | ||
$productUrlPath = $this->urlPathGenerator->getUrlPath($product); | ||
$productUrlRewrite = $this->urlRewriteGenerator->generate($product); | ||
$product->unsUrlPath(); | ||
$product->setUrlPath($productUrlPath); | ||
|
||
try { | ||
$this->urlPersist->replace($productUrlRewrite); | ||
} catch (UrlAlreadyExistsException $e) { | ||
throw new UrlAlreadyExistsException( | ||
__( | ||
'Can not change the visibility of the product with SKU equals "%1". URL key "%2" for specified store already exists.', | ||
$product->getSku(), | ||
$product->getUrlKey() | ||
), | ||
$e, | ||
$e->getCode(), | ||
$e->getUrls() | ||
); | ||
} | ||
} | ||
} | ||
} | ||
|
||
/** | ||
* Get Product Models by Id's | ||
* | ||
* @param array $productIds | ||
* @return array | ||
*/ | ||
private function getProductsByIds(array $productIds): array | ||
{ | ||
$productCollection = $this->productCollectionFactory->create(); | ||
$productCollection->addAttributeToSelect(ProductInterface::VISIBILITY); | ||
$productCollection->addAttributeToSelect('url_key'); | ||
$productCollection->addFieldToFilter( | ||
'entity_id', | ||
['in' => array_unique($productIds)] | ||
); | ||
|
||
return $productCollection->getItems(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
<?php | ||
/** | ||
* Copyright © Magento, Inc. All rights reserved. | ||
* See COPYING.txt for license details. | ||
*/ | ||
declare(strict_types=1); | ||
|
||
namespace Magento\CatalogUrlRewrite\Observer; | ||
|
||
use Magento\Catalog\Api\Data\ProductInterface; | ||
use Magento\CatalogUrlRewrite\Model\Products\AdaptUrlRewritesToVisibilityAttribute; | ||
use Magento\Framework\Event\Observer; | ||
use Magento\Framework\Event\ObserverInterface; | ||
use Magento\UrlRewrite\Model\Exception\UrlAlreadyExistsException; | ||
|
||
/** | ||
* Consider URL rewrites on change product visibility via mass action | ||
*/ | ||
class ProcessUrlRewriteOnChangeProductVisibilityObserver implements ObserverInterface | ||
{ | ||
/** | ||
* @var AdaptUrlRewritesToVisibilityAttribute | ||
*/ | ||
private $adaptUrlRewritesToVisibility; | ||
|
||
/** | ||
* @param AdaptUrlRewritesToVisibilityAttribute $adaptUrlRewritesToVisibility | ||
*/ | ||
public function __construct(AdaptUrlRewritesToVisibilityAttribute $adaptUrlRewritesToVisibility) | ||
{ | ||
$this->adaptUrlRewritesToVisibility = $adaptUrlRewritesToVisibility; | ||
} | ||
|
||
/** | ||
* Generate urls for UrlRewrites and save it in storage | ||
* | ||
* @param Observer $observer | ||
* @return void | ||
* @throws UrlAlreadyExistsException | ||
*/ | ||
public function execute(Observer $observer) | ||
{ | ||
$event = $observer->getEvent(); | ||
$attrData = $event->getAttributesData(); | ||
$productIds = $event->getProductIds(); | ||
$visibility = $attrData[ProductInterface::VISIBILITY] ?? 0; | ||
|
||
if (!$visibility || !$productIds) { | ||
return; | ||
} | ||
|
||
$this->adaptUrlRewritesToVisibility->execute($productIds, $visibility); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please ensure There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @sivaschenko |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,194 @@ | ||
<?php | ||
/** | ||
* Copyright © Magento, Inc. All rights reserved. | ||
* See COPYING.txt for license details. | ||
*/ | ||
namespace Magento\CatalogUrlRewrite\Observer; | ||
|
||
use Magento\Catalog\Api\Data\ProductInterface; | ||
use Magento\Catalog\Api\ProductRepositoryInterface; | ||
use Magento\Catalog\Model\Product\Visibility; | ||
use Magento\Framework\Event\ManagerInterface; | ||
use Magento\Store\Model\StoreManagerInterface; | ||
use Magento\UrlRewrite\Model\Exception\UrlAlreadyExistsException; | ||
use Magento\UrlRewrite\Service\V1\Data\UrlRewrite; | ||
|
||
/** | ||
* @magentoAppArea adminhtml | ||
* @magentoDbIsolation disabled | ||
*/ | ||
class ProcessUrlRewriteOnChangeVisibilityObserverTest extends \PHPUnit\Framework\TestCase | ||
{ | ||
/** | ||
* @var \Magento\Framework\ObjectManagerInterface | ||
*/ | ||
private $objectManager; | ||
|
||
/** | ||
* @var ProductRepositoryInterface | ||
*/ | ||
private $productRepository; | ||
|
||
/** | ||
* @var ManagerInterface | ||
*/ | ||
private $eventManager; | ||
|
||
/** | ||
* Set up | ||
*/ | ||
protected function setUp() | ||
{ | ||
$this->objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager(); | ||
$this->productRepository = $this->objectManager->create(ProductRepositoryInterface::class); | ||
$this->eventManager = $this->objectManager->create(ManagerInterface::class); | ||
} | ||
|
||
/** | ||
* @magentoDataFixture Magento/CatalogUrlRewrite/_files/product_rewrite_multistore.php | ||
* @magentoAppIsolation enabled | ||
*/ | ||
public function testMakeProductInvisibleViaMassAction() | ||
{ | ||
/** @var \Magento\Catalog\Model\Product $product*/ | ||
$product = $this->productRepository->get('product1'); | ||
|
||
/** @var StoreManagerInterface $storeManager */ | ||
$storeManager = $this->objectManager->get(StoreManagerInterface::class); | ||
$storeManager->setCurrentStore(0); | ||
|
||
$testStore = $storeManager->getStore('test'); | ||
$productFilter = [ | ||
UrlRewrite::ENTITY_TYPE => 'product', | ||
]; | ||
|
||
$expected = [ | ||
[ | ||
'request_path' => "product-1.html", | ||
'target_path' => "catalog/product/view/id/" . $product->getId(), | ||
'is_auto_generated' => 1, | ||
'redirect_type' => 0, | ||
'store_id' => '1', | ||
], | ||
[ | ||
'request_path' => "product-1.html", | ||
'target_path' => "catalog/product/view/id/" . $product->getId(), | ||
'is_auto_generated' => 1, | ||
'redirect_type' => 0, | ||
'store_id' => $testStore->getId(), | ||
] | ||
]; | ||
|
||
$actual = $this->getActualResults($productFilter); | ||
foreach ($expected as $row) { | ||
$this->assertContains($row, $actual); | ||
} | ||
|
||
$this->eventManager->dispatch( | ||
'catalog_product_attribute_update_before', | ||
[ | ||
'attributes_data' => [ ProductInterface::VISIBILITY => Visibility::VISIBILITY_NOT_VISIBLE ], | ||
'product_ids' => [$product->getId()] | ||
] | ||
); | ||
|
||
$actual = $this->getActualResults($productFilter); | ||
$this->assertCount(0, $actual); | ||
} | ||
|
||
/** | ||
* @magentoDataFixture Magento/CatalogUrlRewrite/_files/product_invisible_multistore.php | ||
* @magentoAppIsolation enabled | ||
*/ | ||
public function testMakeProductVisibleViaMassAction() | ||
{ | ||
/** @var \Magento\Catalog\Model\Product $product*/ | ||
$product = $this->productRepository->get('product1'); | ||
|
||
/** @var StoreManagerInterface $storeManager */ | ||
$storeManager = $this->objectManager->get(StoreManagerInterface::class); | ||
$storeManager->setCurrentStore(0); | ||
|
||
$testStore = $storeManager->getStore('test'); | ||
$productFilter = [ | ||
UrlRewrite::ENTITY_TYPE => 'product', | ||
]; | ||
|
||
$actual = $this->getActualResults($productFilter); | ||
$this->assertCount(0, $actual); | ||
|
||
$this->eventManager->dispatch( | ||
'catalog_product_attribute_update_before', | ||
[ | ||
'attributes_data' => [ ProductInterface::VISIBILITY => Visibility::VISIBILITY_BOTH ], | ||
'product_ids' => [$product->getId()] | ||
] | ||
); | ||
|
||
$expected = [ | ||
[ | ||
'request_path' => "product-1.html", | ||
'target_path' => "catalog/product/view/id/" . $product->getId(), | ||
'is_auto_generated' => 1, | ||
'redirect_type' => 0, | ||
'store_id' => '1', | ||
], | ||
[ | ||
'request_path' => "product-1.html", | ||
'target_path' => "catalog/product/view/id/" . $product->getId(), | ||
'is_auto_generated' => 1, | ||
'redirect_type' => 0, | ||
'store_id' => $testStore->getId(), | ||
] | ||
]; | ||
|
||
$actual = $this->getActualResults($productFilter); | ||
foreach ($expected as $row) { | ||
$this->assertContains($row, $actual); | ||
} | ||
} | ||
|
||
/** | ||
* @magentoDataFixture Magento/CatalogUrlRewrite/_files/products_invisible.php | ||
* @magentoAppIsolation enabled | ||
*/ | ||
public function testErrorOnDuplicatedUrlKey() | ||
{ | ||
$skus = ['product1', 'product2']; | ||
foreach ($skus as $sku) { | ||
/** @var \Magento\Catalog\Model\Product $product */ | ||
$productIds[] = $this->productRepository->get($sku)->getId(); | ||
} | ||
$this->expectException(UrlAlreadyExistsException::class); | ||
$this->expectExceptionMessage('Can not change the visibility of the product with SKU equals "product2". URL key "product-1" for specified store already exists.'); | ||
|
||
$this->eventManager->dispatch( | ||
'catalog_product_attribute_update_before', | ||
[ | ||
'attributes_data' => [ ProductInterface::VISIBILITY => Visibility::VISIBILITY_BOTH ], | ||
'product_ids' => $productIds | ||
] | ||
); | ||
} | ||
|
||
/** | ||
* @param array $filter | ||
* @return array | ||
*/ | ||
private function getActualResults(array $filter) | ||
{ | ||
/** @var \Magento\UrlRewrite\Model\UrlFinderInterface $urlFinder */ | ||
$urlFinder = $this->objectManager->get(\Magento\UrlRewrite\Model\UrlFinderInterface::class); | ||
$actualResults = []; | ||
foreach ($urlFinder->findAllByData($filter) as $url) { | ||
$actualResults[] = [ | ||
'request_path' => $url->getRequestPath(), | ||
'target_path' => $url->getTargetPath(), | ||
'is_auto_generated' => (int)$url->getIsAutogenerated(), | ||
'redirect_type' => $url->getRedirectType(), | ||
'store_id' => $url->getStoreId() | ||
]; | ||
} | ||
return $actualResults; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should not touch visibility here as the point of whole model is to adapt the rewrite to visibility changes and not change the visibility?