-
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
Merged
magento-engcom-team
merged 11 commits into
magento:2.3-develop
from
VitaliyBoyko:20434-consider-url-rewrite-when-change-product-visibility-attribute-2-3
Mar 26, 2019
Merged
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
24382e3
ENGCOM-20434: Consider deleting/creating url rewrites on change the p…
4d18c9a
ENGCOM-20434: Added accidentally removed code
6a5776c
ENGCOM-20434: Removed url key validation for invisible products
f2088d9
ENGCOM-20434: Removed url key validation for invisible products (reve…
4edec38
ENGCOM-20434: Removed url key validation for invisible products (reve…
a8d1dc0
ENGCOM-20434: static fixes
5687704
Merge branch '2.3-develop' of github.com:magento/magento2 into 20434-…
c865bdc
ENGCOM-20434: refactoring, test coverage
1c608d7
ENGCOM-20434: refactoring, doc blocks adjustments
a80943f
ENGCOM-20434: fixed type inconsistency
a8d9be1
ENGCOM-4357: Static tests fix.
p-bystritsky File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
126 changes: 126 additions & 0 deletions
126
app/code/Magento/CatalogUrlRewrite/Model/Products/AdaptUrlRewritesToVisibilityAttribute.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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(); | ||
} | ||
} |
54 changes: 54 additions & 0 deletions
54
...Magento/CatalogUrlRewrite/Observer/ProcessUrlRewriteOnChangeProductVisibilityObserver.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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, (int)$visibility); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
194 changes: 194 additions & 0 deletions
194
...te/Magento/CatalogUrlRewrite/Observer/ProcessUrlRewriteOnChangeVisibilityObserverTest.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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; | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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?