From 0f92be1c21bdab12be3f588611b32754792aeccc Mon Sep 17 00:00:00 2001 From: Graham Wharton Date: Tue, 25 Feb 2020 15:32:42 +0000 Subject: [PATCH 01/11] Fixed URL Rewrite generation and removal on product/website change. Moved scope of ProductToWebsiteChangeObserver to base instead of adminhtml as this functionality has been moved to a consumer. With the observer set to adminhtml, this observer was not being called properly on changing the websites for a particular product in the consumer. Corrected ProductProcessUrlRewriteSavingObserver to correctly add and remove rewrite URLs when products are added and removed from websites using the Product Edit Admin Page. Corrected ProductToWebsiteChangeObserver to correctly add and remove rewrite URLs when products are added and removed using the Products Grid Mass Action function. Extended unit tests to cover both Observers. Fixed MFTF test for Product URL Rewrite generation. --- .../Magento/Catalog/Model/Product/Action.php | 10 +- ...ProductProcessUrlRewriteSavingObserver.php | 44 ++- .../ProductToWebsiteChangeObserver.php | 41 +-- ...uctProcessUrlRewriteSavingObserverTest.php | 146 +++++++--- .../ProductToWebsiteChangeObserverTest.php | 271 ++++++++++++++++++ .../etc/adminhtml/events.xml | 12 - .../Magento/CatalogUrlRewrite/etc/events.xml | 3 + .../Model/ProductUrlRewriteTest.php | 3 + 8 files changed, 465 insertions(+), 65 deletions(-) create mode 100644 app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/ProductToWebsiteChangeObserverTest.php delete mode 100644 app/code/Magento/CatalogUrlRewrite/etc/adminhtml/events.xml diff --git a/app/code/Magento/Catalog/Model/Product/Action.php b/app/code/Magento/Catalog/Model/Product/Action.php index 3863cf2457247..aba82e458a693 100644 --- a/app/code/Magento/Catalog/Model/Product/Action.php +++ b/app/code/Magento/Catalog/Model/Product/Action.php @@ -169,6 +169,14 @@ public function updateWebsites($productIds, $websiteIds, $type) $categoryIndexer->reindexList(array_unique($productIds)); } - $this->_eventManager->dispatch('catalog_product_to_website_change', ['products' => $productIds]); + //Dispatch event to update Rewrite URLs for new/removed websites + $this->_eventManager->dispatch( + 'catalog_product_to_website_change', + [ + 'products' => $productIds, + 'website_ids' => $websiteIds, + 'action_type' => $type + ] + ); } } diff --git a/app/code/Magento/CatalogUrlRewrite/Observer/ProductProcessUrlRewriteSavingObserver.php b/app/code/Magento/CatalogUrlRewrite/Observer/ProductProcessUrlRewriteSavingObserver.php index 6eda8dd0b61ee..ded4bfbaa04ed 100644 --- a/app/code/Magento/CatalogUrlRewrite/Observer/ProductProcessUrlRewriteSavingObserver.php +++ b/app/code/Magento/CatalogUrlRewrite/Observer/ProductProcessUrlRewriteSavingObserver.php @@ -11,9 +11,17 @@ use Magento\Framework\App\ObjectManager; use Magento\UrlRewrite\Model\UrlPersistInterface; use Magento\Framework\Event\ObserverInterface; +use Magento\Catalog\Model\Product\Visibility; +use Magento\Store\Model\StoreManagerInterface; +use Magento\UrlRewrite\Service\V1\Data\UrlRewrite; +use Magento\Store\Api\StoreWebsiteRelationInterface; /** * Class ProductProcessUrlRewriteSavingObserver + * + * Observer to update the Rewrite URLs for a product. + * This observer is triggered on the save function when making changes + * to the products website on the Product Edit page. */ class ProductProcessUrlRewriteSavingObserver implements ObserverInterface { @@ -32,20 +40,38 @@ class ProductProcessUrlRewriteSavingObserver implements ObserverInterface */ private $productUrlPathGenerator; + /** + * @var StoreManagerInterface $storeManager + */ + private $storeManager; + + /** + * @var StoreWebsiteRelationInterface + */ + private $storeWebsiteRelation; + /** * @param ProductUrlRewriteGenerator $productUrlRewriteGenerator * @param UrlPersistInterface $urlPersist * @param ProductUrlPathGenerator|null $productUrlPathGenerator + * @param StoreManagerInterface|null $storeManager + * @param StoreWebsiteRelationInterface|null $storeWebsiteRelation */ public function __construct( ProductUrlRewriteGenerator $productUrlRewriteGenerator, UrlPersistInterface $urlPersist, - ProductUrlPathGenerator $productUrlPathGenerator = null + ProductUrlPathGenerator $productUrlPathGenerator = null, + StoreManagerInterface $storeManager = null, + StoreWebsiteRelationInterface $storeWebsiteRelation = null ) { $this->productUrlRewriteGenerator = $productUrlRewriteGenerator; $this->urlPersist = $urlPersist; $this->productUrlPathGenerator = $productUrlPathGenerator ?: ObjectManager::getInstance() ->get(ProductUrlPathGenerator::class); + $this->storeManager = $storeManager ?: ObjectManager::getInstance() + ->get(StoreManagerInterface::class); + $this->storeWebsiteRelation = $storeWebsiteRelation ?: ObjectManager::getInstance() + ->get(StoreWebsiteRelationInterface::class); } /** @@ -65,10 +91,24 @@ public function execute(\Magento\Framework\Event\Observer $observer) || $product->getIsChangedWebsites() || $product->dataHasChangedFor('visibility') ) { - if ($product->isVisibleInSiteVisibility()) { + if ($product->getVisibility() != Visibility::VISIBILITY_NOT_VISIBLE) { $product->unsUrlPath(); $product->setUrlPath($this->productUrlPathGenerator->getUrlPath($product)); $this->urlPersist->replace($this->productUrlRewriteGenerator->generate($product)); + + //Remove any rewrite URLs for websites the product is not in + foreach ($this->storeManager->getWebsites() as $website) { + $websiteId = $website->getWebsiteId(); + if (!in_array($websiteId, $product->getWebsiteIds())) { + foreach ($this->storeWebsiteRelation->getStoreByWebsiteId($websiteId) as $storeId) { + $this->urlPersist->deleteByData([ + UrlRewrite::ENTITY_ID => $product->getId(), + UrlRewrite::ENTITY_TYPE => ProductUrlRewriteGenerator::ENTITY_TYPE, + UrlRewrite::STORE_ID => $storeId + ]); + } + } + } } } } diff --git a/app/code/Magento/CatalogUrlRewrite/Observer/ProductToWebsiteChangeObserver.php b/app/code/Magento/CatalogUrlRewrite/Observer/ProductToWebsiteChangeObserver.php index 44b47faf3d4b8..45cec45a3c75d 100644 --- a/app/code/Magento/CatalogUrlRewrite/Observer/ProductToWebsiteChangeObserver.php +++ b/app/code/Magento/CatalogUrlRewrite/Observer/ProductToWebsiteChangeObserver.php @@ -17,7 +17,11 @@ use Magento\Framework\App\ObjectManager; /** - * Observer to assign the products to website + * Class ProductToWebsiteChangeObserver + * + * Observer to update the Rewrite URLs for a product. + * This observer is triggered by the product_action_attribute.website.update + * consumer in response to Mass Action changes in the Admin Product Grid. */ class ProductToWebsiteChangeObserver implements ObserverInterface { @@ -36,11 +40,6 @@ class ProductToWebsiteChangeObserver implements ObserverInterface */ protected $productRepository; - /** - * @var RequestInterface - */ - protected $request; - /** * @var StoreWebsiteRelationInterface */ @@ -52,6 +51,8 @@ class ProductToWebsiteChangeObserver implements ObserverInterface * @param ProductRepositoryInterface $productRepository * @param RequestInterface $request * @param StoreWebsiteRelationInterface $storeWebsiteRelation + * + * * @SuppressWarnings(PHPMD.UnusedFormalParameter) */ public function __construct( ProductUrlRewriteGenerator $productUrlRewriteGenerator, @@ -63,7 +64,6 @@ public function __construct( $this->productUrlRewriteGenerator = $productUrlRewriteGenerator; $this->urlPersist = $urlPersist; $this->productRepository = $productRepository; - $this->request = $request; $this->storeWebsiteRelation = $storeWebsiteRelation ?: ObjectManager::getInstance()->get(StoreWebsiteRelationInterface::class); } @@ -77,24 +77,29 @@ public function __construct( public function execute(\Magento\Framework\Event\Observer $observer) { foreach ($observer->getEvent()->getProducts() as $productId) { + /* @var \Magento\Catalog\Model\Product $product */ $product = $this->productRepository->getById( $productId, false, - $this->request->getParam('store_id', Store::DEFAULT_STORE_ID) + Store::DEFAULT_STORE_ID, + true ); - if (!empty($this->productUrlRewriteGenerator->generate($product))) { - if ($this->request->getParam('remove_website_ids')) { - foreach ($this->request->getParam('remove_website_ids') as $webId) { - foreach ($this->storeWebsiteRelation->getStoreByWebsiteId($webId) as $storeId) { - $this->urlPersist->deleteByData([ - UrlRewrite::ENTITY_ID => $product->getId(), - UrlRewrite::ENTITY_TYPE => ProductUrlRewriteGenerator::ENTITY_TYPE, - UrlRewrite::STORE_ID => $storeId - ]); - } + // Remove the URLs from websites this product no longer belongs to + if ($observer->getEvent()->getActionType() == "remove" && $observer->getEvent()->getWebsiteIds()) { + foreach ($observer->getEvent()->getWebsiteIds() as $webId) { + foreach ($this->storeWebsiteRelation->getStoreByWebsiteId($webId) as $storeId) { + $this->urlPersist->deleteByData([ + UrlRewrite::ENTITY_ID => $product->getId(), + UrlRewrite::ENTITY_TYPE => ProductUrlRewriteGenerator::ENTITY_TYPE, + UrlRewrite::STORE_ID => $storeId + ]); } } + } + + // Refresh all existing URLs for the product + if (!empty($this->productUrlRewriteGenerator->generate($product))) { if ($product->getVisibility() != Visibility::VISIBILITY_NOT_VISIBLE) { $this->urlPersist->replace($this->productUrlRewriteGenerator->generate($product)); } diff --git a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/ProductProcessUrlRewriteSavingObserverTest.php b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/ProductProcessUrlRewriteSavingObserverTest.php index 39317b42af989..f68eaa2a02b9f 100644 --- a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/ProductProcessUrlRewriteSavingObserverTest.php +++ b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/ProductProcessUrlRewriteSavingObserverTest.php @@ -8,38 +8,51 @@ use Magento\CatalogUrlRewrite\Model\ProductUrlRewriteGenerator; use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; -use Magento\UrlRewrite\Service\V1\Data\UrlRewrite; +use Magento\Store\Api\StoreWebsiteRelationInterface; +use Magento\Store\Model\Store; +use Magento\UrlRewrite\Model\UrlPersistInterface; +use Magento\Catalog\Model\Product; +use Magento\Framework\Event; +use Magento\Framework\Event\Observer; +use Magento\Store\Model\StoreManagerInterface; +use Magento\Store\Model\Website; +use Magento\CatalogUrlRewrite\Observer\ProductProcessUrlRewriteSavingObserver; +use Magento\Catalog\Model\Product\Visibility; /** * Class ProductProcessUrlRewriteSavingObserverTest * + * Tests the ProductProcessUrlRewriteSavingObserver to ensure the + * replace method (refresh existing URLs) and deleteByData (remove + * old URLs) are called the correct number of times. + * * @SuppressWarnings(PHPMD.TooManyFields) * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ class ProductProcessUrlRewriteSavingObserverTest extends \PHPUnit\Framework\TestCase { /** - * @var \Magento\UrlRewrite\Model\UrlPersistInterface|\PHPUnit_Framework_MockObject_MockObject + * @var UrlPersistInterface|\PHPUnit_Framework_MockObject_MockObject */ protected $urlPersist; /** - * @var \Magento\Framework\Event|\PHPUnit_Framework_MockObject_MockObject + * @var Event|\PHPUnit_Framework_MockObject_MockObject */ protected $event; /** - * @var \Magento\Framework\Event\Observer|\PHPUnit_Framework_MockObject_MockObject + * @var Observer|\PHPUnit_Framework_MockObject_MockObject */ protected $observer; /** - * @var \Magento\Catalog\Model\Product|\PHPUnit_Framework_MockObject_MockObject + * @var Product|\PHPUnit_Framework_MockObject_MockObject */ protected $product; /** - * @var \Magento\CatalogUrlRewrite\Model\ProductUrlRewriteGenerator|\PHPUnit_Framework_MockObject_MockObject + * @var ProductUrlRewriteGenerator|\PHPUnit_Framework_MockObject_MockObject */ protected $productUrlRewriteGenerator; @@ -49,42 +62,87 @@ class ProductProcessUrlRewriteSavingObserverTest extends \PHPUnit\Framework\Test protected $objectManager; /** - * @var \Magento\CatalogUrlRewrite\Observer\ProductProcessUrlRewriteSavingObserver + * @var ProductProcessUrlRewriteSavingObserver */ protected $model; + /** + * @var StoreManagerInterface|\PHPUnit_Framework_MockObject_MockObject + */ + protected $storeManager; + + /** + * @var Website|\PHPUnit_Framework_MockObject_MockObject + */ + protected $website1; + + /** + * @var Website|\PHPUnit_Framework_MockObject_MockObject + */ + protected $website2; + + /** + * @var StoreWebsiteRelationInterface|\PHPUnit_Framework_MockObject_MockObject + */ + private $storeWebsiteRelation; + /** * Set up + * Website_ID = 1 -> Store_ID = 1 + * Website_ID = 2 -> Store_ID = 2 & 5 */ protected function setUp() { - $this->urlPersist = $this->createMock(\Magento\UrlRewrite\Model\UrlPersistInterface::class); - $this->product = $this->createPartialMock(\Magento\Catalog\Model\Product::class, [ + $this->urlPersist = $this->createMock(UrlPersistInterface::class); + $this->product = $this->createPartialMock( + Product::class, + [ 'getId', 'dataHasChangedFor', - 'isVisibleInSiteVisibility', + 'getVisibility', 'getIsChangedWebsites', 'getIsChangedCategories', - 'getStoreId' - ]); + 'getStoreId', + 'getWebsiteIds' + ] + ); $this->product->expects($this->any())->method('getId')->will($this->returnValue(3)); - $this->event = $this->createPartialMock(\Magento\Framework\Event::class, ['getProduct']); + $this->event = $this->createPartialMock(Event::class, ['getProduct']); $this->event->expects($this->any())->method('getProduct')->willReturn($this->product); - $this->observer = $this->createPartialMock(\Magento\Framework\Event\Observer::class, ['getEvent']); + $this->observer = $this->createPartialMock(Observer::class, ['getEvent']); $this->observer->expects($this->any())->method('getEvent')->willReturn($this->event); $this->productUrlRewriteGenerator = $this->createPartialMock( - \Magento\CatalogUrlRewrite\Model\ProductUrlRewriteGenerator::class, + ProductUrlRewriteGenerator::class, ['generate'] ); $this->productUrlRewriteGenerator->expects($this->any()) ->method('generate') ->will($this->returnValue([3 => 'rewrite'])); $this->objectManager = new ObjectManager($this); + $this->storeManager = $this->createMock(StoreManagerInterface::class); + $this->website1 = $this->createPartialMock(Website::class, ['getWebsiteId']); + $this->website1->expects($this->any())->method('getWebsiteId')->willReturn(1); + $this->website2 = $this->createPartialMock(Website::class, ['getWebsiteId']); + $this->website2->expects($this->any())->method('getWebsiteId')->willReturn(2); + $this->storeManager->expects($this->any()) + ->method('getWebsites') + ->will($this->returnValue([$this->website1, $this->website2])); + + $this->storeWebsiteRelation = $this->createPartialMock( + StoreWebsiteRelationInterface::class, + ['getStoreByWebsiteId'] + ); + $this->storeWebsiteRelation->expects($this->any()) + ->method('getStoreByWebsiteId') + ->will($this->returnValueMap([[1, [1]], [2, [2, 5]]])); + $this->model = $this->objectManager->getObject( - \Magento\CatalogUrlRewrite\Observer\ProductProcessUrlRewriteSavingObserver::class, + ProductProcessUrlRewriteSavingObserver::class, [ 'productUrlRewriteGenerator' => $this->productUrlRewriteGenerator, - 'urlPersist' => $this->urlPersist + 'urlPersist' => $this->urlPersist, + 'storeManager' => $this->storeManager, + 'storeWebsiteRelation' => $this->storeWebsiteRelation ] ); } @@ -102,49 +160,61 @@ public function urlKeyDataProvider() 'isChangedVisibility' => false, 'isChangedWebsites' => false, 'isChangedCategories' => false, - 'visibilityResult' => true, + 'visibilityResult' => Visibility::VISIBILITY_BOTH, 'expectedReplaceCount' => 1, + 'expectedDeleteCount' => 2, + 'productInWebsites' => [1] ], - 'no chnages' => [ + 'no changes' => [ 'isChangedUrlKey' => false, 'isChangedVisibility' => false, 'isChangedWebsites' => false, 'isChangedCategories' => false, - 'visibilityResult' => true, - 'expectedReplaceCount' => 0 + 'visibilityResult' => Visibility::VISIBILITY_BOTH, + 'expectedReplaceCount' => 0, + 'expectedDeleteCount' => 0, + 'productInWebsites' => [1, 2] ], 'visibility changed' => [ 'isChangedUrlKey' => false, 'isChangedVisibility' => true, 'isChangedWebsites' => false, 'isChangedCategories' => false, - 'visibilityResult' => true, - 'expectedReplaceCount' => 1 + 'visibilityResult' => Visibility::VISIBILITY_BOTH, + 'expectedReplaceCount' => 1, + 'expectedDeleteCount' => 0, + 'productInWebsites' => [1, 2] ], 'websites changed' => [ 'isChangedUrlKey' => false, 'isChangedVisibility' => false, 'isChangedWebsites' => true, 'isChangedCategories' => false, - 'visibilityResult' => true, - 'expectedReplaceCount' => 1 + 'visibilityResult' => Visibility::VISIBILITY_BOTH, + 'expectedReplaceCount' => 1, + 'expectedDeleteCount' => 0, + 'productInWebsites' => [1, 2] ], 'categories changed' => [ 'isChangedUrlKey' => false, 'isChangedVisibility' => false, 'isChangedWebsites' => false, 'isChangedCategories' => true, - 'visibilityResult' => true, - 'expectedReplaceCount' => 1 + 'visibilityResult' => Visibility::VISIBILITY_BOTH, + 'expectedReplaceCount' => 1, + 'expectedDeleteCount' => 0, + 'productInWebsites' => [1, 2] ], 'url changed invisible' => [ 'isChangedUrlKey' => true, 'isChangedVisibility' => false, 'isChangedWebsites' => false, 'isChangedCategories' => false, - 'visibilityResult' => false, - 'expectedReplaceCount' => 0 + 'visibilityResult' => Visibility::VISIBILITY_NOT_VISIBLE, + 'expectedReplaceCount' => 0, + 'expectedDeleteCount' => 0, + 'productInWebsites' => [1, 2] ], ]; } @@ -156,6 +226,8 @@ public function urlKeyDataProvider() * @param bool $isChangedCategories * @param bool $visibilityResult * @param int $expectedReplaceCount + * @param int $expectedDeleteCount + * @param int $productInWebsites * * @dataProvider urlKeyDataProvider */ @@ -165,9 +237,16 @@ public function testExecuteUrlKey( $isChangedWebsites, $isChangedCategories, $visibilityResult, - $expectedReplaceCount + $expectedReplaceCount, + $expectedDeleteCount, + $productInWebsites ) { - $this->product->expects($this->any())->method('getStoreId')->will($this->returnValue(12)); + $this->product->expects($this->any())->method('getStoreId')->will( + $this->returnValue(Store::DEFAULT_STORE_ID) + ); + $this->product->expects($this->any())->method('getWebsiteIds')->will( + $this->returnValue($productInWebsites) + ); $this->product->expects($this->any()) ->method('dataHasChangedFor') @@ -187,13 +266,16 @@ public function testExecuteUrlKey( ->will($this->returnValue($isChangedCategories)); $this->product->expects($this->any()) - ->method('isVisibleInSiteVisibility') + ->method('getVisibility') ->will($this->returnValue($visibilityResult)); $this->urlPersist->expects($this->exactly($expectedReplaceCount)) ->method('replace') ->with([3 => 'rewrite']); + $this->urlPersist->expects($this->exactly($expectedDeleteCount)) + ->method('deleteByData'); + $this->model->execute($this->observer); } } diff --git a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/ProductToWebsiteChangeObserverTest.php b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/ProductToWebsiteChangeObserverTest.php new file mode 100644 index 0000000000000..04bf14974b6e7 --- /dev/null +++ b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/ProductToWebsiteChangeObserverTest.php @@ -0,0 +1,271 @@ + Store_ID = 1 + * Website_ID = 2 -> Store_ID = 2 & 5 + */ + protected function setUp() + { + $this->urlPersist = $this->createMock(UrlPersistInterface::class); + $this->productUrlRewriteGenerator = $this->createPartialMock( + ProductUrlRewriteGenerator::class, + ['generate'] + ); + + $this->storeManager = $this->createMock(StoreManagerInterface::class); + $this->website1 = $this->createPartialMock(Website::class, ['getWebsiteId']); + $this->website1->expects($this->any())->method('getWebsiteId')->willReturn(1); + $this->website2 = $this->createPartialMock(Website::class, ['getWebsiteId']); + $this->website2->expects($this->any())->method('getWebsiteId')->willReturn(2); + $this->storeManager->expects($this->any()) + ->method('getWebsites') + ->will($this->returnValue([$this->website1, $this->website2])); + + $this->storeWebsiteRelation = $this->createPartialMock( + StoreWebsiteRelationInterface::class, + ['getStoreByWebsiteId'] + ); + $this->storeWebsiteRelation->expects($this->any()) + ->method('getStoreByWebsiteId') + ->will($this->returnValueMap([[1, [1]], [2, [2, 5]]])); + + $this->product = $this->createPartialMock( + Product::class, + ['getId', 'getVisibility', 'getStoreId', 'getWebsiteIds'] + ); + $this->product->expects($this->any())->method('getId')->will($this->returnValue(3)); + $this->productRepository = $this->getMockForAbstractClass(ProductRepositoryInterface::class); + $this->productRepository->expects($this->any())->method('getById')->willReturn($this->product); + $this->event = $this->createPartialMock( + Event::class, + ['getProducts', 'getActionType', 'getWebsiteIds'] + ); + $this->event->expects($this->any())->method('getProducts')->willReturn([$this->product]); + $this->observer = $this->createPartialMock(Observer::class, ['getEvent']); + $this->observer->expects($this->any())->method('getEvent')->willReturn($this->event); + + $this->objectManager = new ObjectManager($this); + $this->model = $this->objectManager->getObject( + ProductToWebsiteChangeObserver::class, + [ + 'productUrlRewriteGenerator' => $this->productUrlRewriteGenerator, + 'urlPersist' => $this->urlPersist, + 'productRepository' => $this->productRepository, + 'storeWebsiteRelation' => $this->storeWebsiteRelation + ] + ); + } + + /** + * Data provider + * + * @return array + */ + public function urlKeyDataProvider() + { + return [ + 'in_all_no_changes' => [ + 'visibilityResult' => Visibility::VISIBILITY_BOTH, + 'expectedReplaceCount' => 1, + 'expectedDeleteCount' => 0, + 'productInWebsites' => [1, 2], + 'actionType' => null, + 'websitesChanged' => [], + 'rewrites' => [1 => 'url', 2 => 'url', 5 => 'url'] + ], + 'add_to_website_from_empty' => [ + 'visibilityResult' => Visibility::VISIBILITY_BOTH, + 'expectedReplaceCount' => 1, + 'expectedDeleteCount' => 0, + 'productInWebsites' => [1, 2], + 'actionType' => 'add', + 'websitesChanged' => [1, 2], + 'rewrites' => [1 => 'url', 2 => 'url', 5 => 'url'] + ], + 'add_to_website_existing' => [ + 'visibilityResult' => Visibility::VISIBILITY_BOTH, + 'expectedReplaceCount' => 1, + 'expectedDeleteCount' => 0, + 'productInWebsites' => [1, 2], + 'actionType' => 'add', + 'websitesChanged' => [2], + 'rewrites' => [1 => 'url', 2 => 'url', 5 => 'url'] + ], + 'remove_single' => [ + 'visibilityResult' => Visibility::VISIBILITY_BOTH, + 'expectedReplaceCount' => 1, + 'expectedDeleteCount' => 2, + 'productInWebsites' => [1], + 'actionType' => 'remove', + 'websitesChanged' => [2], + 'rewrites' => [1 => 'url'] + ], + 'remove_all' => [ + 'visibilityResult' => Visibility::VISIBILITY_BOTH, + 'expectedReplaceCount' => 0, + 'expectedDeleteCount' => 3, + 'productInWebsites' => [], + 'actionType' => 'remove', + 'websitesChanged' => [1, 2], + 'rewrites' => [] + ], + 'not_visible_add' => [ + 'visibilityResult' => Visibility::VISIBILITY_NOT_VISIBLE, + 'expectedReplaceCount' => 0, + 'expectedDeleteCount' => 0, + 'productInWebsites' => [1, 2], + 'actionType' => 'add', + 'websitesChanged' => [1, 2], + 'rewrites' => [] + ], + 'not_visible_remove' => [ + 'visibilityResult' => Visibility::VISIBILITY_NOT_VISIBLE, + 'expectedReplaceCount' => 0, + 'expectedDeleteCount' => 3, + 'productInWebsites' => [], + 'actionType' => 'remove', + 'websitesChanged' => [1, 2], + 'rewrites' => [] + ], + ]; + } + + /** + * @param bool $visibilityResult + * @param int $expectedReplaceCount + * @param int $expectedDeleteCount + * @param int $productInWebsites + * @param string $actionType + * @param array $websitesChanged + * @param array $rewrites + * + * @dataProvider urlKeyDataProvider + */ + public function testExecuteUrlKey( + $visibilityResult, + $expectedReplaceCount, + $expectedDeleteCount, + $productInWebsites, + $actionType, + $websitesChanged, + $rewrites + ) { + $this->product->expects($this->any())->method('getStoreId')->will( + $this->returnValue(Store::DEFAULT_STORE_ID) + ); + $this->product->expects($this->any())->method('getWebsiteIds')->will( + $this->returnValue($productInWebsites) + ); + $this->event->expects($this->any())->method('getActionType')->willReturn($actionType); + $this->event->expects($this->any())->method('getWebsiteIds')->willReturn($websitesChanged); + + $this->productUrlRewriteGenerator->expects($this->any()) + ->method('generate') + ->will($this->returnValue($rewrites)); + + $this->product->expects($this->any()) + ->method('getVisibility') + ->will($this->returnValue($visibilityResult)); + + $this->urlPersist->expects($this->exactly($expectedReplaceCount)) + ->method('replace') + ->with($rewrites); + + $this->urlPersist->expects($this->exactly($expectedDeleteCount)) + ->method('deleteByData'); + + $this->model->execute($this->observer); + } +} diff --git a/app/code/Magento/CatalogUrlRewrite/etc/adminhtml/events.xml b/app/code/Magento/CatalogUrlRewrite/etc/adminhtml/events.xml deleted file mode 100644 index 9c4a8aaf41231..0000000000000 --- a/app/code/Magento/CatalogUrlRewrite/etc/adminhtml/events.xml +++ /dev/null @@ -1,12 +0,0 @@ - - - - - - - diff --git a/app/code/Magento/CatalogUrlRewrite/etc/events.xml b/app/code/Magento/CatalogUrlRewrite/etc/events.xml index 728442acf7a44..a6e5a5265b6c8 100644 --- a/app/code/Magento/CatalogUrlRewrite/etc/events.xml +++ b/app/code/Magento/CatalogUrlRewrite/etc/events.xml @@ -36,4 +36,7 @@ + + + diff --git a/dev/tests/integration/testsuite/Magento/CatalogUrlRewrite/Model/ProductUrlRewriteTest.php b/dev/tests/integration/testsuite/Magento/CatalogUrlRewrite/Model/ProductUrlRewriteTest.php index f8fe68c2e0a2d..b0ecd58745444 100644 --- a/dev/tests/integration/testsuite/Magento/CatalogUrlRewrite/Model/ProductUrlRewriteTest.php +++ b/dev/tests/integration/testsuite/Magento/CatalogUrlRewrite/Model/ProductUrlRewriteTest.php @@ -85,6 +85,7 @@ public function productDataProvider(): array 'sku' => 'test-product', 'name' => 'test product', 'price' => 150, + 'website_ids' => [1] ], 'expected_data' => [ [ @@ -104,6 +105,7 @@ public function productDataProvider(): array 'name' => 'test product', 'price' => 150, 'url_key' => 'test-product-url-key', + 'website_ids' => [1] ], 'expected_data' => [ [ @@ -123,6 +125,7 @@ public function productDataProvider(): array 'name' => 'test product', 'price' => 150, 'url_key' => 'test-product-url-key', + 'website_ids' => [1] ], 'expected_data' => [], ], From d3d3f2921b7bc68c958cf1cfb2eaec4948420cf0 Mon Sep 17 00:00:00 2001 From: Graham Wharton Date: Wed, 4 Mar 2020 13:05:26 +0000 Subject: [PATCH 02/11] Processed review comments --- .../Magento/Catalog/Model/Product/Action.php | 10 +- ...ProductProcessUrlRewriteSavingObserver.php | 108 ++-- .../ProductToWebsiteChangeObserver.php | 109 ---- ...ProductProcessUrlRewriteRemovingPlugin.php | 124 +++++ ...uctProcessUrlRewriteSavingObserverTest.php | 227 ++++++-- .../ProductToWebsiteChangeObserverTest.php | 271 ---------- ...uctProcessUrlRewriteRemovingPluginTest.php | 339 ++++++++++++ app/code/Magento/CatalogUrlRewrite/etc/di.xml | 4 + .../Magento/CatalogUrlRewrite/etc/events.xml | 3 - .../UrlRewrite/Model/Storage/DbStorage.php | 29 ++ ...ateURLRewriteWhenCategoryIsDeletedTest.xml | 1 + ...lKeyForStoreViewAndMovingCategory2Test.xml | 2 +- ...rlKeyForStoreViewAndMovingCategoryTest.xml | 2 +- ...CategoryUrlRewriteAndAddNoRedirectTest.xml | 1 + ...yUrlRewriteAndAddPermanentRedirectTest.xml | 1 + ...yUrlRewriteAndAddTemporaryRedirectTest.xml | 1 + ...eUrlRewriteAndAddPermanentRedirectTest.xml | 1 + ...eUrlRewriteAndAddTemporaryRedirectTest.xml | 1 + ...yUrlRewriteAndAddPermanentRedirectTest.xml | 1 + ...tUrlRewriteAndAddTemporaryRedirectTest.xml | 1 + ...eProductURLRewriteAndAddNoRedirectTest.xml | 1 + ...ithCategoryAndAddTemporaryRedirectTest.xml | 1 + ...tUrLRewriteAndAddPermanentRedirectTest.xml | 1 + ...tUrLRewriteAndAddTemporaryRedirectTest.xml | 1 + ...nDeleteCMSPageNoRedirectUrlRewriteTest.xml | 1 + ...CMSPagePermanentRedirectUrlRewriteTest.xml | 1 + ...CMSPageTemporaryRedirectUrlRewriteTest.xml | 1 + .../AdminDeleteCategoryUrlRewriteTest.xml | 1 + ...teCmsPageUrlRewriteWithNoRedirectsTest.xml | 1 + ...ageUrlRewriteWithPermanentRedirectTest.xml | 1 + ...ageUrlRewriteWithTemporaryRedirectTest.xml | 1 + .../Test/AdminDeleteProductUrlRewriteTest.xml | 1 + ...inMarketingUrlRewritesNavigateMenuTest.xml | 1 + ...oryUrlRewriteAndAddAspxRequestPathTest.xml | 1 + ...CategoryUrlRewriteAndAddNoRedirectTest.xml | 1 + ...yUrlRewriteAndAddPermanentRedirectTest.xml | 1 + ...yUrlRewriteAndAddTemporaryRedirectTest.xml | 1 + ...CmsPageRewriteEntityWithNoRedirectTest.xml | 1 + ...eRewriteEntityWithPermanentReirectTest.xml | 1 + ...RewriteEntityWithTemporaryRedirectTest.xml | 1 + ...eCmsPageUrlRewriteAndAddNoRedirectTest.xml | 1 + ...eUrlRewriteAndAddPermanentRedirectTest.xml | 1 + ...eUrlRewriteAndAddTemporaryRedirectTest.xml | 1 + ...tUrlRewriteAndAddTemporaryRedirectTest.xml | 1 + ...uctProcessUrlRewriteSavingObserverTest.php | 486 ++++++++++++++++-- 45 files changed, 1223 insertions(+), 523 deletions(-) delete mode 100644 app/code/Magento/CatalogUrlRewrite/Observer/ProductToWebsiteChangeObserver.php create mode 100644 app/code/Magento/CatalogUrlRewrite/Plugin/ProductProcessUrlRewriteRemovingPlugin.php delete mode 100644 app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/ProductToWebsiteChangeObserverTest.php create mode 100644 app/code/Magento/CatalogUrlRewrite/Test/Unit/Plugin/ProductProcessUrlRewriteRemovingPluginTest.php diff --git a/app/code/Magento/Catalog/Model/Product/Action.php b/app/code/Magento/Catalog/Model/Product/Action.php index aba82e458a693..3863cf2457247 100644 --- a/app/code/Magento/Catalog/Model/Product/Action.php +++ b/app/code/Magento/Catalog/Model/Product/Action.php @@ -169,14 +169,6 @@ public function updateWebsites($productIds, $websiteIds, $type) $categoryIndexer->reindexList(array_unique($productIds)); } - //Dispatch event to update Rewrite URLs for new/removed websites - $this->_eventManager->dispatch( - 'catalog_product_to_website_change', - [ - 'products' => $productIds, - 'website_ids' => $websiteIds, - 'action_type' => $type - ] - ); + $this->_eventManager->dispatch('catalog_product_to_website_change', ['products' => $productIds]); } } diff --git a/app/code/Magento/CatalogUrlRewrite/Observer/ProductProcessUrlRewriteSavingObserver.php b/app/code/Magento/CatalogUrlRewrite/Observer/ProductProcessUrlRewriteSavingObserver.php index ded4bfbaa04ed..7cb916b6a6dd8 100644 --- a/app/code/Magento/CatalogUrlRewrite/Observer/ProductProcessUrlRewriteSavingObserver.php +++ b/app/code/Magento/CatalogUrlRewrite/Observer/ProductProcessUrlRewriteSavingObserver.php @@ -6,15 +6,19 @@ namespace Magento\CatalogUrlRewrite\Observer; use Magento\Catalog\Model\Product; +use Magento\Catalog\Model\ProductRepository; +use Magento\CatalogUrlRewrite\Model\ProductScopeRewriteGenerator; use Magento\CatalogUrlRewrite\Model\ProductUrlPathGenerator; use Magento\CatalogUrlRewrite\Model\ProductUrlRewriteGenerator; -use Magento\Framework\App\ObjectManager; +use Magento\Framework\Event\Observer; +use Magento\Framework\Exception\NoSuchEntityException; +use Magento\UrlRewrite\Model\Exception\UrlAlreadyExistsException; use Magento\UrlRewrite\Model\UrlPersistInterface; use Magento\Framework\Event\ObserverInterface; -use Magento\Catalog\Model\Product\Visibility; use Magento\Store\Model\StoreManagerInterface; -use Magento\UrlRewrite\Service\V1\Data\UrlRewrite; use Magento\Store\Api\StoreWebsiteRelationInterface; +use Magento\UrlRewrite\Model\Storage\DbStorage; +use Magento\Store\Model\Store; /** * Class ProductProcessUrlRewriteSavingObserver @@ -50,38 +54,61 @@ class ProductProcessUrlRewriteSavingObserver implements ObserverInterface */ private $storeWebsiteRelation; + /** + * @var ProductRepository $productRepository + */ + private $productRepository; + + /** + * @var ProductScopeRewriteGenerator + */ + private $productScopeRewriteGenerator; + + /** + * @var DbStorage + */ + private $dbStorage; + /** * @param ProductUrlRewriteGenerator $productUrlRewriteGenerator * @param UrlPersistInterface $urlPersist - * @param ProductUrlPathGenerator|null $productUrlPathGenerator - * @param StoreManagerInterface|null $storeManager - * @param StoreWebsiteRelationInterface|null $storeWebsiteRelation + * @param ProductUrlPathGenerator $productUrlPathGenerator + * @param StoreManagerInterface $storeManager + * @param StoreWebsiteRelationInterface $storeWebsiteRelation + * @param ProductRepository $productRepository + * @param ProductScopeRewriteGenerator $productScopeRewriteGenerator + * @param DbStorage $dbStorage */ public function __construct( ProductUrlRewriteGenerator $productUrlRewriteGenerator, UrlPersistInterface $urlPersist, - ProductUrlPathGenerator $productUrlPathGenerator = null, - StoreManagerInterface $storeManager = null, - StoreWebsiteRelationInterface $storeWebsiteRelation = null + ProductUrlPathGenerator $productUrlPathGenerator, + StoreManagerInterface $storeManager, + StoreWebsiteRelationInterface $storeWebsiteRelation, + ProductRepository $productRepository, + ProductScopeRewriteGenerator $productScopeRewriteGenerator, + DbStorage $dbStorage ) { $this->productUrlRewriteGenerator = $productUrlRewriteGenerator; $this->urlPersist = $urlPersist; - $this->productUrlPathGenerator = $productUrlPathGenerator ?: ObjectManager::getInstance() - ->get(ProductUrlPathGenerator::class); - $this->storeManager = $storeManager ?: ObjectManager::getInstance() - ->get(StoreManagerInterface::class); - $this->storeWebsiteRelation = $storeWebsiteRelation ?: ObjectManager::getInstance() - ->get(StoreWebsiteRelationInterface::class); + $this->productUrlPathGenerator = $productUrlPathGenerator; + $this->storeManager = $storeManager; + $this->storeWebsiteRelation = $storeWebsiteRelation; + $this->productRepository = $productRepository; + $this->productScopeRewriteGenerator = $productScopeRewriteGenerator; + $this->dbStorage = $dbStorage; } /** * Generate urls for UrlRewrite and save it in storage * - * @param \Magento\Framework\Event\Observer $observer + * @param Observer $observer * @return void - * @throws \Magento\UrlRewrite\Model\Exception\UrlAlreadyExistsException + * @throws UrlAlreadyExistsException + * @throws NoSuchEntityException + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ - public function execute(\Magento\Framework\Event\Observer $observer) + public function execute(Observer $observer) { /** @var Product $product */ $product = $observer->getEvent()->getProduct(); @@ -91,24 +118,45 @@ public function execute(\Magento\Framework\Event\Observer $observer) || $product->getIsChangedWebsites() || $product->dataHasChangedFor('visibility') ) { - if ($product->getVisibility() != Visibility::VISIBILITY_NOT_VISIBLE) { - $product->unsUrlPath(); - $product->setUrlPath($this->productUrlPathGenerator->getUrlPath($product)); + //Refresh rewrite urls + $product->unsUrlPath(); + $product->setUrlPath($this->productUrlPathGenerator->getUrlPath($product)); + if (!empty($this->productUrlRewriteGenerator->generate($product))) { $this->urlPersist->replace($this->productUrlRewriteGenerator->generate($product)); + } - //Remove any rewrite URLs for websites the product is not in + $storeIdsToRemove = []; + if ($this->productScopeRewriteGenerator->isGlobalScope($product->getStoreId())) { + //Remove any rewrite URLs for websites the product is not in, or is not visible in. Global Scope. foreach ($this->storeManager->getWebsites() as $website) { $websiteId = $website->getWebsiteId(); - if (!in_array($websiteId, $product->getWebsiteIds())) { - foreach ($this->storeWebsiteRelation->getStoreByWebsiteId($websiteId) as $storeId) { - $this->urlPersist->deleteByData([ - UrlRewrite::ENTITY_ID => $product->getId(), - UrlRewrite::ENTITY_TYPE => ProductUrlRewriteGenerator::ENTITY_TYPE, - UrlRewrite::STORE_ID => $storeId - ]); - } + foreach ($this->storeWebsiteRelation->getStoreByWebsiteId($websiteId) as $storeid) { + //Load the product for the store we are processing so we can see if it is visible + $storeProduct = $this->productRepository->getById( + $product->getId(), + false, + $storeid, + true + ); + if (!$storeProduct->isVisibleInSiteVisibility() || + !in_array($websiteId, $product->getWebsiteIds())) { + $storeIdsToRemove[] = $storeid; + }; } } + } else { + //Only remove rewrite for current scope + if (!$product->isVisibleInSiteVisibility() || + !in_array($product->getStoreId(), $product->getStoreIds())) { + $storeIdsToRemove[] = $product->getStoreId(); + } + } + if (count($storeIdsToRemove)) { + $this->dbStorage->deleteEntitiesFromStores( + $storeIdsToRemove, + [$product->getId()], + ProductUrlRewriteGenerator::ENTITY_TYPE + ); } } } diff --git a/app/code/Magento/CatalogUrlRewrite/Observer/ProductToWebsiteChangeObserver.php b/app/code/Magento/CatalogUrlRewrite/Observer/ProductToWebsiteChangeObserver.php deleted file mode 100644 index 45cec45a3c75d..0000000000000 --- a/app/code/Magento/CatalogUrlRewrite/Observer/ProductToWebsiteChangeObserver.php +++ /dev/null @@ -1,109 +0,0 @@ -productUrlRewriteGenerator = $productUrlRewriteGenerator; - $this->urlPersist = $urlPersist; - $this->productRepository = $productRepository; - $this->storeWebsiteRelation = $storeWebsiteRelation ?: - ObjectManager::getInstance()->get(StoreWebsiteRelationInterface::class); - } - - /** - * Generate urls for UrlRewrite and save it in storage - * - * @param \Magento\Framework\Event\Observer $observer - * @return void - */ - public function execute(\Magento\Framework\Event\Observer $observer) - { - foreach ($observer->getEvent()->getProducts() as $productId) { - /* @var \Magento\Catalog\Model\Product $product */ - $product = $this->productRepository->getById( - $productId, - false, - Store::DEFAULT_STORE_ID, - true - ); - - // Remove the URLs from websites this product no longer belongs to - if ($observer->getEvent()->getActionType() == "remove" && $observer->getEvent()->getWebsiteIds()) { - foreach ($observer->getEvent()->getWebsiteIds() as $webId) { - foreach ($this->storeWebsiteRelation->getStoreByWebsiteId($webId) as $storeId) { - $this->urlPersist->deleteByData([ - UrlRewrite::ENTITY_ID => $product->getId(), - UrlRewrite::ENTITY_TYPE => ProductUrlRewriteGenerator::ENTITY_TYPE, - UrlRewrite::STORE_ID => $storeId - ]); - } - } - } - - // Refresh all existing URLs for the product - if (!empty($this->productUrlRewriteGenerator->generate($product))) { - if ($product->getVisibility() != Visibility::VISIBILITY_NOT_VISIBLE) { - $this->urlPersist->replace($this->productUrlRewriteGenerator->generate($product)); - } - } - } - } -} diff --git a/app/code/Magento/CatalogUrlRewrite/Plugin/ProductProcessUrlRewriteRemovingPlugin.php b/app/code/Magento/CatalogUrlRewrite/Plugin/ProductProcessUrlRewriteRemovingPlugin.php new file mode 100644 index 0000000000000..1bec43c4fd75b --- /dev/null +++ b/app/code/Magento/CatalogUrlRewrite/Plugin/ProductProcessUrlRewriteRemovingPlugin.php @@ -0,0 +1,124 @@ +productRepository = $productRepository; + $this->storeWebsiteRelation = $storeWebsiteRelation; + $this->urlPersist = $urlPersist; + $this->productUrlRewriteGenerator = $productUrlRewriteGenerator; + $this->dbStorage = $dbStorage; + } + + /** + * @param Action $subject + * @param null $result + * @param array $productIds + * @param array $websiteIds + * @param string $type + * @return void + * @SuppressWarnings(PHPMD.UnusedFormalParameter) + */ + public function afterUpdateWebsites( + Action $subject, + $result, + $productIds, + $websiteIds, + $type + ) { + foreach ($productIds as $productId) { + /* @var Product $product */ + $product = $this->productRepository->getById( + $productId, + false, + Store::DEFAULT_STORE_ID, + true + ); + + // Refresh all existing URLs for the product + if (!empty($this->productUrlRewriteGenerator->generate($product))) { + if ($product->isVisibleInSiteVisibility()) { + $this->urlPersist->replace($this->productUrlRewriteGenerator->generate($product)); + } + } + } + + $storeIdsToRemove = []; + // Remove the URLs from websites this product no longer belongs to + if ($type == "remove" && $websiteIds && $productIds) { + foreach ($websiteIds as $webId) { + foreach ($this->storeWebsiteRelation->getStoreByWebsiteId($webId) as $storeid) { + $storeIdsToRemove[] = $storeid; + } + } + if (count($storeIdsToRemove)) { + $this->dbStorage->deleteEntitiesFromStores( + $storeIdsToRemove, + $productIds, + ProductUrlRewriteGenerator::ENTITY_TYPE + ); + } + } + } +} diff --git a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/ProductProcessUrlRewriteSavingObserverTest.php b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/ProductProcessUrlRewriteSavingObserverTest.php index f68eaa2a02b9f..df1effb2f2eb8 100644 --- a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/ProductProcessUrlRewriteSavingObserverTest.php +++ b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/ProductProcessUrlRewriteSavingObserverTest.php @@ -6,10 +6,13 @@ namespace Magento\CatalogUrlRewrite\Test\Unit\Observer; +use Magento\Catalog\Model\ProductRepository; +use Magento\CatalogUrlRewrite\Model\ProductScopeRewriteGenerator; use Magento\CatalogUrlRewrite\Model\ProductUrlRewriteGenerator; use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; use Magento\Store\Api\StoreWebsiteRelationInterface; use Magento\Store\Model\Store; +use Magento\UrlRewrite\Model\Storage\DbStorage; use Magento\UrlRewrite\Model\UrlPersistInterface; use Magento\Catalog\Model\Product; use Magento\Framework\Event; @@ -17,7 +20,8 @@ use Magento\Store\Model\StoreManagerInterface; use Magento\Store\Model\Website; use Magento\CatalogUrlRewrite\Observer\ProductProcessUrlRewriteSavingObserver; -use Magento\Catalog\Model\Product\Visibility; +use PHPUnit\Framework\MockObject\MockObject; +use PHPUnit\Framework\TestCase; /** * Class ProductProcessUrlRewriteSavingObserverTest @@ -29,63 +33,93 @@ * @SuppressWarnings(PHPMD.TooManyFields) * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ -class ProductProcessUrlRewriteSavingObserverTest extends \PHPUnit\Framework\TestCase +class ProductProcessUrlRewriteSavingObserverTest extends TestCase { /** - * @var UrlPersistInterface|\PHPUnit_Framework_MockObject_MockObject + * @var UrlPersistInterface|MockObject */ - protected $urlPersist; + private $urlPersist; /** - * @var Event|\PHPUnit_Framework_MockObject_MockObject + * @var Event|MockObject */ - protected $event; + private $event; /** - * @var Observer|\PHPUnit_Framework_MockObject_MockObject + * @var Observer|MockObject */ - protected $observer; + private $observer; /** - * @var Product|\PHPUnit_Framework_MockObject_MockObject + * @var Product|MockObject */ - protected $product; + private $product; /** - * @var ProductUrlRewriteGenerator|\PHPUnit_Framework_MockObject_MockObject + * @var Product|MockObject */ - protected $productUrlRewriteGenerator; + private $product1; + + /** + * @var Product|MockObject + */ + private $product2; + + /** + * @var Product|MockObject + */ + private $product5; + + /** + * @var ProductUrlRewriteGenerator|MockObject + */ + private $productUrlRewriteGenerator; + + /** + * @var ProductScopeRewriteGenerator|MockObject + */ + private $productScopeRewriteGenerator; /** * @var ObjectManager */ - protected $objectManager; + private $objectManager; /** * @var ProductProcessUrlRewriteSavingObserver */ - protected $model; + private $model; /** - * @var StoreManagerInterface|\PHPUnit_Framework_MockObject_MockObject + * @var StoreManagerInterface|MockObject */ - protected $storeManager; + private $storeManager; /** - * @var Website|\PHPUnit_Framework_MockObject_MockObject + * @var Website|MockObject */ - protected $website1; + private $website1; /** - * @var Website|\PHPUnit_Framework_MockObject_MockObject + * @var Website|MockObject */ - protected $website2; + private $website2; /** - * @var StoreWebsiteRelationInterface|\PHPUnit_Framework_MockObject_MockObject + * @var StoreWebsiteRelationInterface|MockObject */ private $storeWebsiteRelation; + /** + * @var ProductRepository|MockObject + */ + private $productRepository; + + /** + * @var DbStorage|MockObject + */ + private $dbStorage; + /** * Set up * Website_ID = 1 -> Store_ID = 1 @@ -99,14 +133,39 @@ protected function setUp() [ 'getId', 'dataHasChangedFor', - 'getVisibility', + 'isVisibleInSiteVisibility', 'getIsChangedWebsites', 'getIsChangedCategories', 'getStoreId', 'getWebsiteIds' ] ); - $this->product->expects($this->any())->method('getId')->will($this->returnValue(3)); + $this->product1 = $this->createPartialMock( + Product::class, + ['getId', 'isVisibleInSiteVisibility'] + ); + $this->product2 = $this->createPartialMock( + Product::class, + ['getId', 'isVisibleInSiteVisibility'] + ); + $this->product5 = $this->createPartialMock( + Product::class, + ['getId', 'isVisibleInSiteVisibility'] + ); + $this->productRepository = $this->createPartialMock(ProductRepository::class, ['getById']); + $this->product->expects($this->any())->method('getId')->will($this->returnValue(1)); + $this->product1->expects($this->any())->method('getId')->will($this->returnValue(1)); + $this->product2->expects($this->any())->method('getId')->will($this->returnValue(1)); + $this->product5->expects($this->any())->method('getId')->will($this->returnValue(1)); + $this->productRepository->expects($this->any()) + ->method('getById') + ->will($this->returnValueMap([ + [1, false, 0, true, $this->product], + [1, false, 1, true, $this->product1], + [1, false, 2, true, $this->product2], + [1, false, 5, true, $this->product5] + ])); + $this->dbStorage = $this->createPartialMock(DbStorage::class, ['deleteEntitiesFromStores']); $this->event = $this->createPartialMock(Event::class, ['getProduct']); $this->event->expects($this->any())->method('getProduct')->willReturn($this->product); $this->observer = $this->createPartialMock(Observer::class, ['getEvent']); @@ -117,7 +176,20 @@ protected function setUp() ); $this->productUrlRewriteGenerator->expects($this->any()) ->method('generate') - ->will($this->returnValue([3 => 'rewrite'])); + ->will($this->returnValue([1 => 'rewrite'])); + $this->productScopeRewriteGenerator = $this->createPartialMock( + ProductScopeRewriteGenerator::class, + ['isGlobalScope'] + ); + $this->productScopeRewriteGenerator->expects($this->any()) + ->method('isGlobalScope') + ->will($this->returnValueMap([ + [null, true], + [0, true], + [1, false], + [2, false], + [5, false], + ])); $this->objectManager = new ObjectManager($this); $this->storeManager = $this->createMock(StoreManagerInterface::class); $this->website1 = $this->createPartialMock(Website::class, ['getWebsiteId']); @@ -142,7 +214,10 @@ protected function setUp() 'productUrlRewriteGenerator' => $this->productUrlRewriteGenerator, 'urlPersist' => $this->urlPersist, 'storeManager' => $this->storeManager, - 'storeWebsiteRelation' => $this->storeWebsiteRelation + 'storeWebsiteRelation' => $this->storeWebsiteRelation, + 'productRepository' => $this->productRepository, + 'dbStorage' => $this->dbStorage, + 'productScopeRewriteGenerator' => $this->productScopeRewriteGenerator ] ); } @@ -160,10 +235,15 @@ public function urlKeyDataProvider() 'isChangedVisibility' => false, 'isChangedWebsites' => false, 'isChangedCategories' => false, - 'visibilityResult' => Visibility::VISIBILITY_BOTH, + 'visibilityResult' => [ + '0' => true, + '1' => true, + '2' => true, + '5' => true + ], + 'productInWebsites' => [1], 'expectedReplaceCount' => 1, - 'expectedDeleteCount' => 2, - 'productInWebsites' => [1] + 'expectedRemoves' => [2, 5], ], 'no changes' => [ @@ -171,50 +251,75 @@ public function urlKeyDataProvider() 'isChangedVisibility' => false, 'isChangedWebsites' => false, 'isChangedCategories' => false, - 'visibilityResult' => Visibility::VISIBILITY_BOTH, + 'visibilityResult' => [ + '0' => true, + '1' => true, + '2' => true, + '5' => true + ], + 'productInWebsites' => [1, 2], 'expectedReplaceCount' => 0, - 'expectedDeleteCount' => 0, - 'productInWebsites' => [1, 2] + 'expectedRemoves' => [], ], 'visibility changed' => [ 'isChangedUrlKey' => false, 'isChangedVisibility' => true, 'isChangedWebsites' => false, 'isChangedCategories' => false, - 'visibilityResult' => Visibility::VISIBILITY_BOTH, + 'visibilityResult' => [ + '0' => true, + '1' => true, + '2' => true, + '5' => true + ], + 'productInWebsites' => [1, 2], 'expectedReplaceCount' => 1, - 'expectedDeleteCount' => 0, - 'productInWebsites' => [1, 2] + 'expectedRemoves' => [], ], 'websites changed' => [ 'isChangedUrlKey' => false, 'isChangedVisibility' => false, 'isChangedWebsites' => true, 'isChangedCategories' => false, - 'visibilityResult' => Visibility::VISIBILITY_BOTH, + 'visibilityResult' => [ + '0' => true, + '1' => true, + '2' => true, + '5' => true + ], + 'productInWebsites' => [1, 2], 'expectedReplaceCount' => 1, - 'expectedDeleteCount' => 0, - 'productInWebsites' => [1, 2] + 'expectedRemoves' => [], ], 'categories changed' => [ 'isChangedUrlKey' => false, 'isChangedVisibility' => false, 'isChangedWebsites' => false, 'isChangedCategories' => true, - 'visibilityResult' => Visibility::VISIBILITY_BOTH, + 'visibilityResult' => [ + '0' => true, + '1' => true, + '2' => true, + '5' => true + ], + 'productInWebsites' => [1, 2], 'expectedReplaceCount' => 1, - 'expectedDeleteCount' => 0, - 'productInWebsites' => [1, 2] + 'expectedRemoves' => [], ], 'url changed invisible' => [ 'isChangedUrlKey' => true, 'isChangedVisibility' => false, 'isChangedWebsites' => false, 'isChangedCategories' => false, - 'visibilityResult' => Visibility::VISIBILITY_NOT_VISIBLE, - 'expectedReplaceCount' => 0, - 'expectedDeleteCount' => 0, - 'productInWebsites' => [1, 2] + 'visibilityResult' => [ + '0' => false, + '1' => false, + '2' => false, + '5' => false + ], + 'productInWebsites' => [1, 2], + 'expectedReplaceCount' => 1, + 'expectedRemoves' => [1,2,5], ], ]; } @@ -224,10 +329,10 @@ public function urlKeyDataProvider() * @param bool $isChangedVisibility * @param bool $isChangedWebsites * @param bool $isChangedCategories - * @param bool $visibilityResult - * @param int $expectedReplaceCount - * @param int $expectedDeleteCount + * @param array $visibilityResult * @param int $productInWebsites + * @param int $expectedReplaceCount + * @param array $expectedRemoves * * @dataProvider urlKeyDataProvider */ @@ -237,9 +342,9 @@ public function testExecuteUrlKey( $isChangedWebsites, $isChangedCategories, $visibilityResult, + $productInWebsites, $expectedReplaceCount, - $expectedDeleteCount, - $productInWebsites + $expectedRemoves ) { $this->product->expects($this->any())->method('getStoreId')->will( $this->returnValue(Store::DEFAULT_STORE_ID) @@ -266,15 +371,29 @@ public function testExecuteUrlKey( ->will($this->returnValue($isChangedCategories)); $this->product->expects($this->any()) - ->method('getVisibility') - ->will($this->returnValue($visibilityResult)); + ->method('isVisibleInSiteVisibility') + ->will($this->returnValue($visibilityResult['0'])); + $this->product1->expects($this->any()) + ->method('isVisibleInSiteVisibility') + ->will($this->returnValue($visibilityResult['1'])); + $this->product2->expects($this->any()) + ->method('isVisibleInSiteVisibility') + ->will($this->returnValue($visibilityResult['2'])); + $this->product5->expects($this->any()) + ->method('isVisibleInSiteVisibility') + ->will($this->returnValue($visibilityResult['5'])); $this->urlPersist->expects($this->exactly($expectedReplaceCount)) ->method('replace') - ->with([3 => 'rewrite']); + ->with([1 => 'rewrite']); - $this->urlPersist->expects($this->exactly($expectedDeleteCount)) - ->method('deleteByData'); + $this->dbStorage->expects($this->any()) + ->method('deleteEntitiesFromStores') + ->with( + $expectedRemoves, + [1], + ProductUrlRewriteGenerator::ENTITY_TYPE + ); $this->model->execute($this->observer); } diff --git a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/ProductToWebsiteChangeObserverTest.php b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/ProductToWebsiteChangeObserverTest.php deleted file mode 100644 index 04bf14974b6e7..0000000000000 --- a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/ProductToWebsiteChangeObserverTest.php +++ /dev/null @@ -1,271 +0,0 @@ - Store_ID = 1 - * Website_ID = 2 -> Store_ID = 2 & 5 - */ - protected function setUp() - { - $this->urlPersist = $this->createMock(UrlPersistInterface::class); - $this->productUrlRewriteGenerator = $this->createPartialMock( - ProductUrlRewriteGenerator::class, - ['generate'] - ); - - $this->storeManager = $this->createMock(StoreManagerInterface::class); - $this->website1 = $this->createPartialMock(Website::class, ['getWebsiteId']); - $this->website1->expects($this->any())->method('getWebsiteId')->willReturn(1); - $this->website2 = $this->createPartialMock(Website::class, ['getWebsiteId']); - $this->website2->expects($this->any())->method('getWebsiteId')->willReturn(2); - $this->storeManager->expects($this->any()) - ->method('getWebsites') - ->will($this->returnValue([$this->website1, $this->website2])); - - $this->storeWebsiteRelation = $this->createPartialMock( - StoreWebsiteRelationInterface::class, - ['getStoreByWebsiteId'] - ); - $this->storeWebsiteRelation->expects($this->any()) - ->method('getStoreByWebsiteId') - ->will($this->returnValueMap([[1, [1]], [2, [2, 5]]])); - - $this->product = $this->createPartialMock( - Product::class, - ['getId', 'getVisibility', 'getStoreId', 'getWebsiteIds'] - ); - $this->product->expects($this->any())->method('getId')->will($this->returnValue(3)); - $this->productRepository = $this->getMockForAbstractClass(ProductRepositoryInterface::class); - $this->productRepository->expects($this->any())->method('getById')->willReturn($this->product); - $this->event = $this->createPartialMock( - Event::class, - ['getProducts', 'getActionType', 'getWebsiteIds'] - ); - $this->event->expects($this->any())->method('getProducts')->willReturn([$this->product]); - $this->observer = $this->createPartialMock(Observer::class, ['getEvent']); - $this->observer->expects($this->any())->method('getEvent')->willReturn($this->event); - - $this->objectManager = new ObjectManager($this); - $this->model = $this->objectManager->getObject( - ProductToWebsiteChangeObserver::class, - [ - 'productUrlRewriteGenerator' => $this->productUrlRewriteGenerator, - 'urlPersist' => $this->urlPersist, - 'productRepository' => $this->productRepository, - 'storeWebsiteRelation' => $this->storeWebsiteRelation - ] - ); - } - - /** - * Data provider - * - * @return array - */ - public function urlKeyDataProvider() - { - return [ - 'in_all_no_changes' => [ - 'visibilityResult' => Visibility::VISIBILITY_BOTH, - 'expectedReplaceCount' => 1, - 'expectedDeleteCount' => 0, - 'productInWebsites' => [1, 2], - 'actionType' => null, - 'websitesChanged' => [], - 'rewrites' => [1 => 'url', 2 => 'url', 5 => 'url'] - ], - 'add_to_website_from_empty' => [ - 'visibilityResult' => Visibility::VISIBILITY_BOTH, - 'expectedReplaceCount' => 1, - 'expectedDeleteCount' => 0, - 'productInWebsites' => [1, 2], - 'actionType' => 'add', - 'websitesChanged' => [1, 2], - 'rewrites' => [1 => 'url', 2 => 'url', 5 => 'url'] - ], - 'add_to_website_existing' => [ - 'visibilityResult' => Visibility::VISIBILITY_BOTH, - 'expectedReplaceCount' => 1, - 'expectedDeleteCount' => 0, - 'productInWebsites' => [1, 2], - 'actionType' => 'add', - 'websitesChanged' => [2], - 'rewrites' => [1 => 'url', 2 => 'url', 5 => 'url'] - ], - 'remove_single' => [ - 'visibilityResult' => Visibility::VISIBILITY_BOTH, - 'expectedReplaceCount' => 1, - 'expectedDeleteCount' => 2, - 'productInWebsites' => [1], - 'actionType' => 'remove', - 'websitesChanged' => [2], - 'rewrites' => [1 => 'url'] - ], - 'remove_all' => [ - 'visibilityResult' => Visibility::VISIBILITY_BOTH, - 'expectedReplaceCount' => 0, - 'expectedDeleteCount' => 3, - 'productInWebsites' => [], - 'actionType' => 'remove', - 'websitesChanged' => [1, 2], - 'rewrites' => [] - ], - 'not_visible_add' => [ - 'visibilityResult' => Visibility::VISIBILITY_NOT_VISIBLE, - 'expectedReplaceCount' => 0, - 'expectedDeleteCount' => 0, - 'productInWebsites' => [1, 2], - 'actionType' => 'add', - 'websitesChanged' => [1, 2], - 'rewrites' => [] - ], - 'not_visible_remove' => [ - 'visibilityResult' => Visibility::VISIBILITY_NOT_VISIBLE, - 'expectedReplaceCount' => 0, - 'expectedDeleteCount' => 3, - 'productInWebsites' => [], - 'actionType' => 'remove', - 'websitesChanged' => [1, 2], - 'rewrites' => [] - ], - ]; - } - - /** - * @param bool $visibilityResult - * @param int $expectedReplaceCount - * @param int $expectedDeleteCount - * @param int $productInWebsites - * @param string $actionType - * @param array $websitesChanged - * @param array $rewrites - * - * @dataProvider urlKeyDataProvider - */ - public function testExecuteUrlKey( - $visibilityResult, - $expectedReplaceCount, - $expectedDeleteCount, - $productInWebsites, - $actionType, - $websitesChanged, - $rewrites - ) { - $this->product->expects($this->any())->method('getStoreId')->will( - $this->returnValue(Store::DEFAULT_STORE_ID) - ); - $this->product->expects($this->any())->method('getWebsiteIds')->will( - $this->returnValue($productInWebsites) - ); - $this->event->expects($this->any())->method('getActionType')->willReturn($actionType); - $this->event->expects($this->any())->method('getWebsiteIds')->willReturn($websitesChanged); - - $this->productUrlRewriteGenerator->expects($this->any()) - ->method('generate') - ->will($this->returnValue($rewrites)); - - $this->product->expects($this->any()) - ->method('getVisibility') - ->will($this->returnValue($visibilityResult)); - - $this->urlPersist->expects($this->exactly($expectedReplaceCount)) - ->method('replace') - ->with($rewrites); - - $this->urlPersist->expects($this->exactly($expectedDeleteCount)) - ->method('deleteByData'); - - $this->model->execute($this->observer); - } -} diff --git a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Plugin/ProductProcessUrlRewriteRemovingPluginTest.php b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Plugin/ProductProcessUrlRewriteRemovingPluginTest.php new file mode 100644 index 0000000000000..1e9d6a911fe14 --- /dev/null +++ b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Plugin/ProductProcessUrlRewriteRemovingPluginTest.php @@ -0,0 +1,339 @@ + Store_ID = 1 + * Website_ID = 2 -> Store_ID = 2 & 5 + */ + protected function setUp() + { + $this->urlPersist = $this->createMock(UrlPersistInterface::class); + $this->productUrlRewriteGenerator = $this->createPartialMock( + ProductUrlRewriteGenerator::class, + ['generate'] + ); + + $this->storeManager = $this->createMock(StoreManagerInterface::class); + $this->website1 = $this->createPartialMock(Website::class, ['getWebsiteId']); + $this->website1->expects($this->any())->method('getWebsiteId')->willReturn(1); + $this->website2 = $this->createPartialMock(Website::class, ['getWebsiteId']); + $this->website2->expects($this->any())->method('getWebsiteId')->willReturn(2); + $this->storeManager->expects($this->any()) + ->method('getWebsites') + ->will($this->returnValue([$this->website1, $this->website2])); + + $this->storeWebsiteRelation = $this->createPartialMock( + StoreWebsiteRelationInterface::class, + ['getStoreByWebsiteId'] + ); + $this->storeWebsiteRelation->expects($this->any()) + ->method('getStoreByWebsiteId') + ->will($this->returnValueMap([[1, [1]], [2, [2, 5]]])); + + $this->product1 = $this->createPartialMock( + Product::class, + ['getWebsiteIds', 'isVisibleInSiteVisibility'] + ); + $this->product2 = $this->createPartialMock( + Product::class, + ['getWebsiteIds', 'isVisibleInSiteVisibility'] + ); + $this->product3 = $this->createPartialMock( + Product::class, + ['getWebsiteIds', 'isVisibleInSiteVisibility'] + ); + + $this->productRepository = $this->getMockForAbstractClass(ProductRepositoryInterface::class); + $this->productRepository->expects($this->any()) + ->method('getById') + ->will($this->returnValueMap([ + [1, false, 0, true, $this->product1], + [2, false, 0, true, $this->product2], + [3, false, 0, true, $this->product3] + ])); + + $this->dbStorage = $this->createPartialMock(DbStorage::class, ['deleteEntitiesFromStores']); + + $this->subject = $this->createMock( + Action::class + ); + + $this->objectManager = new ObjectManager($this); + $this->plugin = $this->objectManager->getObject( + ProductProcessUrlRewriteRemovingPlugin::class, + [ + 'productRepository' => $this->productRepository, + 'storeWebsiteRelation' => $this->storeWebsiteRelation, + 'urlPersist' => $this->urlPersist, + 'productUrlRewriteGenerator' => $this->productUrlRewriteGenerator, + 'dbStorage' => $this->dbStorage + + ] + ); + } + + /** + * Data provider + * + * @return array + */ + public function afterUpdateWebsitesDataProvider() + { + return [ + 'add_new_websites_1' => [ + 'products' => [ + '1' => ['visibilityResult' => true, 'websiteids' => [1]], + '2' => ['visibilityResult' => true, 'websiteids' => [1]], + '3' => ['visibilityResult' => true, 'websiteids' => [1]], + ], + 'productids' => [1,2,3], + 'type' => 'add', + 'websiteids' => [2], + 'expectedReplaceCount' => 3, + 'expectedStoreRemovals' => [], + 'expectedDeleteCount' => 0, + 'rewrites' => [true] + ], + 'add_new_websites_2' => [ + 'products' => [ + '1' => ['visibilityResult' => true, 'websiteids' => [1]], + '2' => ['visibilityResult' => true, 'websiteids' => [1]], + '3' => ['visibilityResult' => true, 'websiteids' => [1]], + ], + 'productids' => [1,2,3], + 'type' => 'add', + 'websiteids' => [2], + 'expectedReplaceCount' => 3, + 'expectedStoreRemovals' => [], + 'expectedDeleteCount' => 0, + 'rewrites' => [true] + ], + 'remove_all' => [ + 'products' => [ + '1' => ['visibilityResult' => true, 'websiteids' => [1,2]], + '2' => ['visibilityResult' => true, 'websiteids' => [1,2]], + '3' => ['visibilityResult' => true, 'websiteids' => [1,2]], + ], + 'productids' => [1,2,3], + 'type' => 'remove', + 'websiteids' => [1,2], + 'expectedReplaceCount' => 0, + 'expectedStoreRemovals' => [1,2,5], + 'expectedDeleteCount' => 1, + 'rewrites' => [] + ], + 'remove_single' => [ + 'products' => [ + '1' => ['visibilityResult' => true, 'websiteids' => [1,2]], + '2' => ['visibilityResult' => true, 'websiteids' => [1,2]], + '3' => ['visibilityResult' => true, 'websiteids' => [1,2]], + ], + 'productids' => [1,2,3], + 'type' => 'remove', + 'websiteids' => [2], + 'expectedReplaceCount' => 0, + 'expectedStoreRemovals' => [2,5], + 'expectedDeleteCount' => 1, + 'rewrites' => [] + ], + 'not_visible_add_1' => [ + 'products' => [ + '1' => ['visibilityResult' => false, 'websiteids' => [1]], + '2' => ['visibilityResult' => false, 'websiteids' => [1]], + '3' => ['visibilityResult' => false, 'websiteids' => [1]], + ], + 'productids' => [1,2,3], + 'type' => 'add', + 'websiteids' => [2], + 'expectedReplaceCount' => 0, + 'expectedStoreRemovals' => [], + 'expectedDeleteCount' => 0, + 'rewrites' => [true] + ], + 'not_visible_add_2' => [ + 'products' => [ + '1' => ['visibilityResult' => false, 'websiteids' => [1]], + '2' => ['visibilityResult' => false, 'websiteids' => [1]], + '3' => ['visibilityResult' => true, 'websiteids' => [1]], + ], + 'productids' => [1,2,3], + 'type' => 'add', + 'websiteids' => [2], + 'expectedReplaceCount' => 1, + 'expectedStoreRemovals' => [], + 'expectedDeleteCount' => 0, + 'rewrites' => [true] + ], + + ]; + } + + /** + * @param array $products + * @param array $productids + * @param string $type + * @param array $websiteids + * @param int $expectedReplaceCount + * @param array $expectedStoreRemovals + * @param int $expectedDeleteCount + * @param array $rewrites + * + * @dataProvider afterUpdateWebsitesDataProvider + */ + public function testAfterUpdateWebsites( + $products, + $productids, + $type, + $websiteids, + $expectedReplaceCount, + $expectedStoreRemovals, + $expectedDeleteCount, + $rewrites + ) { + + $this->productUrlRewriteGenerator->expects($this->any()) + ->method('generate') + ->will($this->returnValue($rewrites)); + + $this->product1->expects($this->any()) + ->method('getWebsiteIds') + ->will($this->returnValue($products['1']['websiteids'])); + $this->product2->expects($this->any()) + ->method('getWebsiteIds') + ->will($this->returnValue($products['2']['websiteids'])); + $this->product3->expects($this->any()) + ->method('getWebsiteIds') + ->will($this->returnValue($products['3']['websiteids'])); + + $this->product1->expects($this->any()) + ->method('isVisibleInSiteVisibility') + ->will($this->returnValue($products['1']['visibilityResult'])); + $this->product2->expects($this->any()) + ->method('isVisibleInSiteVisibility') + ->will($this->returnValue($products['2']['visibilityResult'])); + $this->product3->expects($this->any()) + ->method('isVisibleInSiteVisibility') + ->will($this->returnValue($products['3']['visibilityResult'])); + + $this->urlPersist->expects($this->exactly($expectedReplaceCount)) + ->method('replace') + ->with($rewrites); + + $this->dbStorage->expects($this->exactly($expectedDeleteCount)) + ->method('deleteEntitiesFromStores') + ->with( + $expectedStoreRemovals, + $productids, + ProductUrlRewriteGenerator::ENTITY_TYPE + ); + + $this->plugin->afterUpdateWebsites( + $this->subject, + null, + $productids, + $websiteids, + $type + ); + } +} diff --git a/app/code/Magento/CatalogUrlRewrite/etc/di.xml b/app/code/Magento/CatalogUrlRewrite/etc/di.xml index 5fb7d33546d60..6efaa5dd8517e 100644 --- a/app/code/Magento/CatalogUrlRewrite/etc/di.xml +++ b/app/code/Magento/CatalogUrlRewrite/etc/di.xml @@ -70,4 +70,8 @@ + + + diff --git a/app/code/Magento/CatalogUrlRewrite/etc/events.xml b/app/code/Magento/CatalogUrlRewrite/etc/events.xml index a6e5a5265b6c8..728442acf7a44 100644 --- a/app/code/Magento/CatalogUrlRewrite/etc/events.xml +++ b/app/code/Magento/CatalogUrlRewrite/etc/events.xml @@ -36,7 +36,4 @@ - - - diff --git a/app/code/Magento/UrlRewrite/Model/Storage/DbStorage.php b/app/code/Magento/UrlRewrite/Model/Storage/DbStorage.php index f0e94e8379ad2..a5d23f99bc0cd 100644 --- a/app/code/Magento/UrlRewrite/Model/Storage/DbStorage.php +++ b/app/code/Magento/UrlRewrite/Model/Storage/DbStorage.php @@ -375,4 +375,33 @@ public function deleteByData(array $data) $this->prepareSelect($data)->deleteFromSelect($this->resource->getTableName(self::TABLE_NAME)) ); } + + /** + * Function deleteEntitiesFromStores + * + * Deletes multiple URL Rewrites from database + * + * @param array $store_ids + * @param array $entity_ids + * @param int $entity_type + */ + public function deleteEntitiesFromStores($store_ids, $entity_ids, $entity_type) + { + $select = $this->connection->select(); + $select->from($this->resource->getTableName(self::TABLE_NAME)); + + $select->where( + $this->connection->quoteIdentifier( + UrlRewrite::STORE_ID + ) . ' IN (' . $this->connection->quote($store_ids, 'INTEGER') . ')' . + ' AND ' . $this->connection->quoteIdentifier( + UrlRewrite::ENTITY_ID + ) . ' IN (' . $this->connection->quote($entity_ids, 'INTEGER') . ')' . + ' AND ' . $this->connection->quoteIdentifier( + UrlRewrite::ENTITY_TYPE + ) . ' = ' . $this->connection->quote($entity_type) + ); + $select = $select->deleteFromSelect($this->resource->getTableName(self::TABLE_NAME)); + $this->connection->query($select); + } } diff --git a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminAutoUpdateURLRewriteWhenCategoryIsDeletedTest.xml b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminAutoUpdateURLRewriteWhenCategoryIsDeletedTest.xml index 022d28ed692c1..a5473863b9ba9 100644 --- a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminAutoUpdateURLRewriteWhenCategoryIsDeletedTest.xml +++ b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminAutoUpdateURLRewriteWhenCategoryIsDeletedTest.xml @@ -15,6 +15,7 @@ + diff --git a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCheckUrlRewritesInCatalogCategoriesAfterChangingUrlKeyForStoreViewAndMovingCategory2Test.xml b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCheckUrlRewritesInCatalogCategoriesAfterChangingUrlKeyForStoreViewAndMovingCategory2Test.xml index e14bb5342db91..d26ee18b1c719 100644 --- a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCheckUrlRewritesInCatalogCategoriesAfterChangingUrlKeyForStoreViewAndMovingCategory2Test.xml +++ b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCheckUrlRewritesInCatalogCategoriesAfterChangingUrlKeyForStoreViewAndMovingCategory2Test.xml @@ -16,7 +16,7 @@ - + diff --git a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCheckUrlRewritesInCatalogCategoriesAfterChangingUrlKeyForStoreViewAndMovingCategoryTest.xml b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCheckUrlRewritesInCatalogCategoriesAfterChangingUrlKeyForStoreViewAndMovingCategoryTest.xml index 7f9ee3020c388..30eafbbd39a99 100644 --- a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCheckUrlRewritesInCatalogCategoriesAfterChangingUrlKeyForStoreViewAndMovingCategoryTest.xml +++ b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCheckUrlRewritesInCatalogCategoriesAfterChangingUrlKeyForStoreViewAndMovingCategoryTest.xml @@ -15,7 +15,7 @@ - + Use AdminCheckUrlRewritesInCatalogCategoriesAfterChangingUrlKeyForStoreViewAndMovingCategory2Test instead diff --git a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateCategoryUrlRewriteAndAddNoRedirectTest.xml b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateCategoryUrlRewriteAndAddNoRedirectTest.xml index 732fc22aaf84a..3216eec1a2196 100644 --- a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateCategoryUrlRewriteAndAddNoRedirectTest.xml +++ b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateCategoryUrlRewriteAndAddNoRedirectTest.xml @@ -15,6 +15,7 @@ + diff --git a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateCategoryUrlRewriteAndAddPermanentRedirectTest.xml b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateCategoryUrlRewriteAndAddPermanentRedirectTest.xml index 867b3ee54161c..6ffce05552aae 100644 --- a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateCategoryUrlRewriteAndAddPermanentRedirectTest.xml +++ b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateCategoryUrlRewriteAndAddPermanentRedirectTest.xml @@ -15,6 +15,7 @@ + diff --git a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateCategoryUrlRewriteAndAddTemporaryRedirectTest.xml b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateCategoryUrlRewriteAndAddTemporaryRedirectTest.xml index ab18add56aeb9..7b6c222d65e92 100644 --- a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateCategoryUrlRewriteAndAddTemporaryRedirectTest.xml +++ b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateCategoryUrlRewriteAndAddTemporaryRedirectTest.xml @@ -15,6 +15,7 @@ + diff --git a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateCustomCMSPageUrlRewriteAndAddPermanentRedirectTest.xml b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateCustomCMSPageUrlRewriteAndAddPermanentRedirectTest.xml index bd4f7d7a32165..662913adc4e47 100644 --- a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateCustomCMSPageUrlRewriteAndAddPermanentRedirectTest.xml +++ b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateCustomCMSPageUrlRewriteAndAddPermanentRedirectTest.xml @@ -15,6 +15,7 @@ + diff --git a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateCustomCMSPageUrlRewriteAndAddTemporaryRedirectTest.xml b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateCustomCMSPageUrlRewriteAndAddTemporaryRedirectTest.xml index 074140845c8a6..049402aba12c5 100644 --- a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateCustomCMSPageUrlRewriteAndAddTemporaryRedirectTest.xml +++ b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateCustomCMSPageUrlRewriteAndAddTemporaryRedirectTest.xml @@ -15,6 +15,7 @@ + diff --git a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateCustomCategoryUrlRewriteAndAddPermanentRedirectTest.xml b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateCustomCategoryUrlRewriteAndAddPermanentRedirectTest.xml index cdd7c334e35cd..40fb0b7763723 100644 --- a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateCustomCategoryUrlRewriteAndAddPermanentRedirectTest.xml +++ b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateCustomCategoryUrlRewriteAndAddPermanentRedirectTest.xml @@ -15,6 +15,7 @@ + diff --git a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateCustomProductUrlRewriteAndAddTemporaryRedirectTest.xml b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateCustomProductUrlRewriteAndAddTemporaryRedirectTest.xml index 593c4282fc516..7bb97d7258f2f 100644 --- a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateCustomProductUrlRewriteAndAddTemporaryRedirectTest.xml +++ b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateCustomProductUrlRewriteAndAddTemporaryRedirectTest.xml @@ -15,6 +15,7 @@ + diff --git a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateProductURLRewriteAndAddNoRedirectTest.xml b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateProductURLRewriteAndAddNoRedirectTest.xml index c51030315b287..dccc98d107581 100644 --- a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateProductURLRewriteAndAddNoRedirectTest.xml +++ b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateProductURLRewriteAndAddNoRedirectTest.xml @@ -15,6 +15,7 @@ + diff --git a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateProductURLRewriteWithCategoryAndAddTemporaryRedirectTest.xml b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateProductURLRewriteWithCategoryAndAddTemporaryRedirectTest.xml index d433aa7557094..7edb4ac1eec3f 100644 --- a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateProductURLRewriteWithCategoryAndAddTemporaryRedirectTest.xml +++ b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateProductURLRewriteWithCategoryAndAddTemporaryRedirectTest.xml @@ -15,6 +15,7 @@ + diff --git a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateProductUrLRewriteAndAddPermanentRedirectTest.xml b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateProductUrLRewriteAndAddPermanentRedirectTest.xml index e1ff4f598943e..90ec26c7d1284 100644 --- a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateProductUrLRewriteAndAddPermanentRedirectTest.xml +++ b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateProductUrLRewriteAndAddPermanentRedirectTest.xml @@ -15,6 +15,7 @@ + diff --git a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateProductUrLRewriteAndAddTemporaryRedirectTest.xml b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateProductUrLRewriteAndAddTemporaryRedirectTest.xml index 3fa4e6b7bad90..1023c7c58c3d7 100644 --- a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateProductUrLRewriteAndAddTemporaryRedirectTest.xml +++ b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminCreateProductUrLRewriteAndAddTemporaryRedirectTest.xml @@ -15,6 +15,7 @@ + diff --git a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminDeleteCMSPageNoRedirectUrlRewriteTest.xml b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminDeleteCMSPageNoRedirectUrlRewriteTest.xml index 1171a5e8b79c3..3f7ce95cb05b4 100644 --- a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminDeleteCMSPageNoRedirectUrlRewriteTest.xml +++ b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminDeleteCMSPageNoRedirectUrlRewriteTest.xml @@ -15,6 +15,7 @@ + diff --git a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminDeleteCMSPagePermanentRedirectUrlRewriteTest.xml b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminDeleteCMSPagePermanentRedirectUrlRewriteTest.xml index 566204094cabd..9378d40b5d11c 100644 --- a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminDeleteCMSPagePermanentRedirectUrlRewriteTest.xml +++ b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminDeleteCMSPagePermanentRedirectUrlRewriteTest.xml @@ -15,6 +15,7 @@ + diff --git a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminDeleteCMSPageTemporaryRedirectUrlRewriteTest.xml b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminDeleteCMSPageTemporaryRedirectUrlRewriteTest.xml index 22cb74bf96ad6..85d1f42449377 100644 --- a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminDeleteCMSPageTemporaryRedirectUrlRewriteTest.xml +++ b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminDeleteCMSPageTemporaryRedirectUrlRewriteTest.xml @@ -15,6 +15,7 @@ + diff --git a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminDeleteCategoryUrlRewriteTest.xml b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminDeleteCategoryUrlRewriteTest.xml index ceb71f65e4489..125008e9cf2e0 100644 --- a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminDeleteCategoryUrlRewriteTest.xml +++ b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminDeleteCategoryUrlRewriteTest.xml @@ -14,6 +14,7 @@ + diff --git a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminDeleteCmsPageUrlRewriteWithNoRedirectsTest.xml b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminDeleteCmsPageUrlRewriteWithNoRedirectsTest.xml index e2f0d6af0deab..16a6f4b7562d7 100644 --- a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminDeleteCmsPageUrlRewriteWithNoRedirectsTest.xml +++ b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminDeleteCmsPageUrlRewriteWithNoRedirectsTest.xml @@ -16,6 +16,7 @@ + diff --git a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminDeleteCmsPageUrlRewriteWithPermanentRedirectTest.xml b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminDeleteCmsPageUrlRewriteWithPermanentRedirectTest.xml index 741be6985d517..ccd6175e7cbe7 100644 --- a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminDeleteCmsPageUrlRewriteWithPermanentRedirectTest.xml +++ b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminDeleteCmsPageUrlRewriteWithPermanentRedirectTest.xml @@ -15,6 +15,7 @@ + diff --git a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminDeleteCmsPageUrlRewriteWithTemporaryRedirectTest.xml b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminDeleteCmsPageUrlRewriteWithTemporaryRedirectTest.xml index e3d417f3c1f39..5a873cdb9b5b7 100644 --- a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminDeleteCmsPageUrlRewriteWithTemporaryRedirectTest.xml +++ b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminDeleteCmsPageUrlRewriteWithTemporaryRedirectTest.xml @@ -16,6 +16,7 @@ + diff --git a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminDeleteProductUrlRewriteTest.xml b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminDeleteProductUrlRewriteTest.xml index 8e0ea8334a2cc..211a4b244300a 100644 --- a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminDeleteProductUrlRewriteTest.xml +++ b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminDeleteProductUrlRewriteTest.xml @@ -14,6 +14,7 @@ + diff --git a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminMarketingUrlRewritesNavigateMenuTest.xml b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminMarketingUrlRewritesNavigateMenuTest.xml index 443307b427b42..18a6ef868ff51 100644 --- a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminMarketingUrlRewritesNavigateMenuTest.xml +++ b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminMarketingUrlRewritesNavigateMenuTest.xml @@ -18,6 +18,7 @@ + diff --git a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCategoryUrlRewriteAndAddAspxRequestPathTest.xml b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCategoryUrlRewriteAndAddAspxRequestPathTest.xml index fb8f200741c8a..ace9f3f9b0abb 100644 --- a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCategoryUrlRewriteAndAddAspxRequestPathTest.xml +++ b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCategoryUrlRewriteAndAddAspxRequestPathTest.xml @@ -15,6 +15,7 @@ + diff --git a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCategoryUrlRewriteAndAddNoRedirectTest.xml b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCategoryUrlRewriteAndAddNoRedirectTest.xml index 72180105b38f8..66c1b5ae20103 100644 --- a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCategoryUrlRewriteAndAddNoRedirectTest.xml +++ b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCategoryUrlRewriteAndAddNoRedirectTest.xml @@ -15,6 +15,7 @@ + diff --git a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCategoryUrlRewriteAndAddPermanentRedirectTest.xml b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCategoryUrlRewriteAndAddPermanentRedirectTest.xml index 3fb8e5da39caf..014f5de510bde 100644 --- a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCategoryUrlRewriteAndAddPermanentRedirectTest.xml +++ b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCategoryUrlRewriteAndAddPermanentRedirectTest.xml @@ -15,6 +15,7 @@ + diff --git a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCategoryUrlRewriteAndAddTemporaryRedirectTest.xml b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCategoryUrlRewriteAndAddTemporaryRedirectTest.xml index bea5c44461a70..794c3991f2b6e 100644 --- a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCategoryUrlRewriteAndAddTemporaryRedirectTest.xml +++ b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCategoryUrlRewriteAndAddTemporaryRedirectTest.xml @@ -15,6 +15,7 @@ + diff --git a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCmsPageRewriteEntityWithNoRedirectTest.xml b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCmsPageRewriteEntityWithNoRedirectTest.xml index b2fa13ead1164..90e0e03293556 100644 --- a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCmsPageRewriteEntityWithNoRedirectTest.xml +++ b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCmsPageRewriteEntityWithNoRedirectTest.xml @@ -15,6 +15,7 @@ + diff --git a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCmsPageRewriteEntityWithPermanentReirectTest.xml b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCmsPageRewriteEntityWithPermanentReirectTest.xml index 3bf278db8410a..4186bf61d2298 100644 --- a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCmsPageRewriteEntityWithPermanentReirectTest.xml +++ b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCmsPageRewriteEntityWithPermanentReirectTest.xml @@ -14,6 +14,7 @@ + diff --git a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCmsPageRewriteEntityWithTemporaryRedirectTest.xml b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCmsPageRewriteEntityWithTemporaryRedirectTest.xml index b00241bc3acac..0057ec91934bf 100644 --- a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCmsPageRewriteEntityWithTemporaryRedirectTest.xml +++ b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCmsPageRewriteEntityWithTemporaryRedirectTest.xml @@ -15,6 +15,7 @@ + diff --git a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCmsPageUrlRewriteAndAddNoRedirectTest.xml b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCmsPageUrlRewriteAndAddNoRedirectTest.xml index b799a58ac9e40..6dba13d8f0679 100644 --- a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCmsPageUrlRewriteAndAddNoRedirectTest.xml +++ b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCmsPageUrlRewriteAndAddNoRedirectTest.xml @@ -15,6 +15,7 @@ + diff --git a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCmsPageUrlRewriteAndAddPermanentRedirectTest.xml b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCmsPageUrlRewriteAndAddPermanentRedirectTest.xml index 3cf30444fcaee..dd8c93c50480a 100644 --- a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCmsPageUrlRewriteAndAddPermanentRedirectTest.xml +++ b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCmsPageUrlRewriteAndAddPermanentRedirectTest.xml @@ -15,6 +15,7 @@ + diff --git a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCmsPageUrlRewriteAndAddTemporaryRedirectTest.xml b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCmsPageUrlRewriteAndAddTemporaryRedirectTest.xml index b95d1eaa44d7f..dba869196dc13 100644 --- a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCmsPageUrlRewriteAndAddTemporaryRedirectTest.xml +++ b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateCmsPageUrlRewriteAndAddTemporaryRedirectTest.xml @@ -15,6 +15,7 @@ + diff --git a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateProductUrlRewriteAndAddTemporaryRedirectTest.xml b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateProductUrlRewriteAndAddTemporaryRedirectTest.xml index 74f3a60f35cea..b80dc86e48c8e 100644 --- a/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateProductUrlRewriteAndAddTemporaryRedirectTest.xml +++ b/app/code/Magento/UrlRewrite/Test/Mftf/Test/AdminUpdateProductUrlRewriteAndAddTemporaryRedirectTest.xml @@ -15,6 +15,7 @@ + diff --git a/dev/tests/integration/testsuite/Magento/CatalogUrlRewrite/Observer/ProductProcessUrlRewriteSavingObserverTest.php b/dev/tests/integration/testsuite/Magento/CatalogUrlRewrite/Observer/ProductProcessUrlRewriteSavingObserverTest.php index c72a58197b1fd..eae90a47cead3 100644 --- a/dev/tests/integration/testsuite/Magento/CatalogUrlRewrite/Observer/ProductProcessUrlRewriteSavingObserverTest.php +++ b/dev/tests/integration/testsuite/Magento/CatalogUrlRewrite/Observer/ProductProcessUrlRewriteSavingObserverTest.php @@ -5,25 +5,46 @@ */ namespace Magento\CatalogUrlRewrite\Observer; +use Magento\Catalog\Api\ProductRepositoryInterface; +use Magento\Catalog\Model\Product; +use Magento\Framework\ObjectManagerInterface; use Magento\Store\Model\StoreManagerInterface; +use Magento\TestFramework\Helper\Bootstrap; +use Magento\UrlRewrite\Model\UrlFinderInterface; use Magento\UrlRewrite\Service\V1\Data\UrlRewrite; -use Magento\CatalogUrlRewrite\Model\CategoryUrlRewriteGenerator; +use Magento\Catalog\Model\Product\Visibility; +use Magento\Store\Model\Store; +use PHPUnit\Framework\TestCase; /** * @magentoAppArea adminhtml * @magentoDbIsolation disabled */ -class ProductProcessUrlRewriteSavingObserverTest extends \PHPUnit\Framework\TestCase +class ProductProcessUrlRewriteSavingObserverTest extends TestCase { - /** @var \Magento\Framework\ObjectManagerInterface */ - protected $objectManager; + /** + * @var ObjectManagerInterface + */ + private $objectManager; + + /** + * @var StoreManagerInterface $storeManager + */ + private $storeManager; + + /** + * @var ProductRepositoryInterface $productRepository + */ + private $productRepository; /** * Set up */ protected function setUp() { - $this->objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager(); + $this->objectManager = Bootstrap::getObjectManager(); + $this->storeManager = $this->objectManager->get(StoreManagerInterface::class); + $this->productRepository = $this->objectManager->get(ProductRepositoryInterface::class); } /** @@ -32,8 +53,8 @@ protected function setUp() */ private function getActualResults(array $filter) { - /** @var \Magento\UrlRewrite\Model\UrlFinderInterface $urlFinder */ - $urlFinder = $this->objectManager->get(\Magento\UrlRewrite\Model\UrlFinderInterface::class); + /** @var UrlFinderInterface $urlFinder */ + $urlFinder = $this->objectManager->get(UrlFinderInterface::class); $actualResults = []; foreach ($urlFinder->findAllByData($filter) as $url) { $actualResults[] = [ @@ -53,16 +74,14 @@ private function getActualResults(array $filter) */ public function testUrlKeyHasChangedInGlobalContext() { - /** @var \Magento\Catalog\Api\ProductRepositoryInterface $productRepository*/ - $productRepository = $this->objectManager->create(\Magento\Catalog\Api\ProductRepositoryInterface::class); - /** @var \Magento\Catalog\Model\Product $product*/ - $product = $productRepository->get('product1'); + $testStore1 = $this->storeManager->getStore('default'); + $testStore4 = $this->storeManager->getStore('test'); - /** @var StoreManagerInterface $storeManager */ - $storeManager = $this->objectManager->get(StoreManagerInterface::class); - $storeManager->setCurrentStore(0); + $this->storeManager->setCurrentStore(Store::DEFAULT_STORE_ID); + + /** @var Product $product*/ + $product = $this->productRepository->get('product1'); - $testStore = $storeManager->getStore('test'); $productFilter = [ UrlRewrite::ENTITY_TYPE => 'product', ]; @@ -73,14 +92,14 @@ public function testUrlKeyHasChangedInGlobalContext() 'target_path' => "catalog/product/view/id/" . $product->getId(), 'is_auto_generated' => 1, 'redirect_type' => 0, - 'store_id' => 1, + 'store_id' => $testStore1->getId(), ], [ 'request_path' => "product-1.html", 'target_path' => "catalog/product/view/id/" . $product->getId(), 'is_auto_generated' => 1, 'redirect_type' => 0, - 'store_id' => $testStore->getId(), + 'store_id' => $testStore4->getId(), ], ]; $actual = $this->getActualResults($productFilter); @@ -91,7 +110,7 @@ public function testUrlKeyHasChangedInGlobalContext() $product->setData('save_rewrites_history', true); $product->setUrlKey('new-url'); $product->setUrlPath('new-path'); - $product->save(); + $this->productRepository->save($product); $expected = [ [ @@ -99,28 +118,28 @@ public function testUrlKeyHasChangedInGlobalContext() 'target_path' => "catalog/product/view/id/" . $product->getId(), 'is_auto_generated' => 1, 'redirect_type' => 0, - 'store_id' => 1, + 'store_id' => $testStore1->getId(), ], [ 'request_path' => "new-url.html", 'target_path' => "catalog/product/view/id/" . $product->getId(), 'is_auto_generated' => 1, 'redirect_type' => 0, - 'store_id' => $testStore->getId(), + 'store_id' => $testStore4->getId(), ], [ 'request_path' => "product-1.html", 'target_path' => "new-url.html", 'is_auto_generated' => 0, 'redirect_type' => 301, - 'store_id' => 1, + 'store_id' => $testStore1->getId(), ], [ 'request_path' => "product-1.html", 'target_path' => "new-url.html", 'is_auto_generated' => 0, 'redirect_type' => 301, - 'store_id' => $testStore->getId(), + 'store_id' => $testStore4->getId(), ], ]; @@ -136,16 +155,13 @@ public function testUrlKeyHasChangedInGlobalContext() */ public function testUrlKeyHasChangedInStoreviewContextWithPermanentRedirection() { - /** @var \Magento\Catalog\Api\ProductRepositoryInterface $productRepository*/ - $productRepository = $this->objectManager->create(\Magento\Catalog\Api\ProductRepositoryInterface::class); - /** @var \Magento\Catalog\Model\Product $product*/ - $product = $productRepository->get('product1'); + $testStore1 = $this->storeManager->getStore('default'); + $testStore4 = $this->storeManager->getStore('test'); - /** @var StoreManagerInterface $storeManager */ - $storeManager = $this->objectManager->get(StoreManagerInterface::class); - $storeManager->setCurrentStore(1); + $this->storeManager->setCurrentStore($testStore1); - $testStore = $storeManager->getStore('test'); + /** @var Product $product*/ + $product = $this->productRepository->get('product1'); $productFilter = [ UrlRewrite::ENTITY_TYPE => 'product', @@ -154,7 +170,7 @@ public function testUrlKeyHasChangedInStoreviewContextWithPermanentRedirection() $product->setData('save_rewrites_history', true); $product->setUrlKey('new-url'); $product->setUrlPath('new-path'); - $product->save(); + $this->productRepository->save($product); $expected = [ [ @@ -162,21 +178,21 @@ public function testUrlKeyHasChangedInStoreviewContextWithPermanentRedirection() 'target_path' => "catalog/product/view/id/" . $product->getId(), 'is_auto_generated' => 1, 'redirect_type' => 0, - 'store_id' => 1, + 'store_id' => $testStore1->getId(), ], [ 'request_path' => "product-1.html", 'target_path' => "catalog/product/view/id/" . $product->getId(), 'is_auto_generated' => 1, 'redirect_type' => 0, - 'store_id' => $testStore->getId(), + 'store_id' => $testStore4->getId(), ], [ 'request_path' => "product-1.html", 'target_path' => "new-url.html", 'is_auto_generated' => 0, 'redirect_type' => 301, - 'store_id' => 1, + 'store_id' => $testStore1->getId(), ], ]; @@ -192,16 +208,13 @@ public function testUrlKeyHasChangedInStoreviewContextWithPermanentRedirection() */ public function testUrlKeyHasChangedInStoreviewContextWithoutPermanentRedirection() { - /** @var \Magento\Catalog\Api\ProductRepositoryInterface $productRepository*/ - $productRepository = $this->objectManager->create(\Magento\Catalog\Api\ProductRepositoryInterface::class); - /** @var \Magento\Catalog\Model\Product $product*/ - $product = $productRepository->get('product1'); + $testStore1 = $this->storeManager->getStore('default'); + $testStore4 = $this->storeManager->getStore('test'); - /** @var StoreManagerInterface $storeManager */ - $storeManager = $this->objectManager->get(StoreManagerInterface::class); - $storeManager->setCurrentStore(1); + $this->storeManager->setCurrentStore(1); - $testStore = $storeManager->getStore('test'); + /** @var Product $product*/ + $product = $this->productRepository->get('product1'); $productFilter = [ UrlRewrite::ENTITY_TYPE => 'product', @@ -210,7 +223,7 @@ public function testUrlKeyHasChangedInStoreviewContextWithoutPermanentRedirectio $product->setData('save_rewrites_history', false); $product->setUrlKey('new-url'); $product->setUrlPath('new-path'); - $product->save(); + $this->productRepository->save($product); $expected = [ [ @@ -218,17 +231,400 @@ public function testUrlKeyHasChangedInStoreviewContextWithoutPermanentRedirectio 'target_path' => "catalog/product/view/id/" . $product->getId(), 'is_auto_generated' => 1, 'redirect_type' => 0, - 'store_id' => 1, + 'store_id' => $testStore1->getId(), + ], + [ + 'request_path' => "product-1.html", + 'target_path' => "catalog/product/view/id/" . $product->getId(), + 'is_auto_generated' => 1, + 'redirect_type' => 0, + 'store_id' => $testStore4->getId(), + ], + ]; + + $actual = $this->getActualResults($productFilter); + foreach ($expected as $row) { + $this->assertContains($row, $actual); + } + } + + /** + * @magentoDataFixture Magento/Store/_files/second_website_with_two_stores.php + * @magentoDataFixture Magento/CatalogUrlRewrite/_files/product_rewrite_multistore.php + * @magentoAppIsolation enabled + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) + */ + public function testAddAndRemoveProductFromWebsite() + { + $testStore1 = $this->storeManager->getStore('default'); + $testStore2 = $this->storeManager->getStore('fixture_second_store'); + $testStore3 = $this->storeManager->getStore('fixture_third_store'); + $testStore4 = $this->storeManager->getStore('test'); + + $this->storeManager->setCurrentStore(Store::DEFAULT_STORE_ID); + + /** @var Product $product*/ + $product = $this->productRepository->get('product1'); + + $productFilter = [ + UrlRewrite::ENTITY_TYPE => 'product', + ]; + + //Product in 1st website. Should result in being in 1st and 4th stores. + $expected = [ + [ + 'request_path' => "product-1.html", + 'target_path' => "catalog/product/view/id/" . $product->getId(), + 'is_auto_generated' => 1, + 'redirect_type' => 0, + 'store_id' => $testStore1->getId(), + ], + [ + 'request_path' => "product-1.html", + 'target_path' => "catalog/product/view/id/" . $product->getId(), + 'is_auto_generated' => 1, + 'redirect_type' => 0, + 'store_id' => $testStore4->getId(), + ] + ]; + $actual = $this->getActualResults($productFilter); + foreach ($expected as $row) { + $this->assertContains($row, $actual); + } + + //Add product to websites corresponding to all 4 stores. + //Rewrites should not be present as the product is hidden + //at the global scope. + $product->setWebsiteIds( + array_unique( + [ + $testStore1->getWebsiteId(), + $testStore2->getWebsiteId(), + $testStore3->getWebsiteId(), + $testStore4->getWebsiteId() + ] + ) + ); + $this->productRepository->save($product); + $expected = [ + [ + 'request_path' => "product-1.html", + 'target_path' => "catalog/product/view/id/" . $product->getId(), + 'is_auto_generated' => 1, + 'redirect_type' => 0, + 'store_id' => $testStore1->getId(), + ], + [ + 'request_path' => "product-1.html", + 'target_path' => "catalog/product/view/id/" . $product->getId(), + 'is_auto_generated' => 1, + 'redirect_type' => 0, + 'store_id' => $testStore2->getId(), + ], + [ + 'request_path' => "product-1.html", + 'target_path' => "catalog/product/view/id/" . $product->getId(), + 'is_auto_generated' => 1, + 'redirect_type' => 0, + 'store_id' => $testStore3->getId(), + ], + [ + 'request_path' => "product-1.html", + 'target_path' => "catalog/product/view/id/" . $product->getId(), + 'is_auto_generated' => 1, + 'redirect_type' => 0, + 'store_id' => $testStore4->getId(), + ] + ]; + + //Add product to websites corresponding to stores 2 and 3. + $product->setWebsiteIds( + array_unique( + [ + $testStore2->getWebsiteId(), + $testStore3->getWebsiteId(), + ] + ) + ); + $this->productRepository->save($product); + $expected = [ + [ + 'request_path' => "product-1.html", + 'target_path' => "catalog/product/view/id/" . $product->getId(), + 'is_auto_generated' => 1, + 'redirect_type' => 0, + 'store_id' => $testStore2->getId(), + ], + [ + 'request_path' => "product-1.html", + 'target_path' => "catalog/product/view/id/" . $product->getId(), + 'is_auto_generated' => 1, + 'redirect_type' => 0, + 'store_id' => $testStore3->getId(), + ] + ]; + $actual = $this->getActualResults($productFilter); + foreach ($expected as $row) { + $this->assertContains($row, $actual); + } + } + + /** + * @magentoDataFixture Magento/Store/_files/second_website_with_two_stores.php + * @magentoDataFixture Magento/CatalogUrlRewrite/_files/product_rewrite_multistore.php + * @magentoAppIsolation enabled + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) + */ + public function testChangeVisibilityGlobalScope() + { + $testStore1 = $this->storeManager->getStore('default'); + $testStore2 = $this->storeManager->getStore('fixture_second_store'); + $testStore3 = $this->storeManager->getStore('fixture_third_store'); + $testStore4 = $this->storeManager->getStore('test'); + + $this->storeManager->setCurrentStore(Store::DEFAULT_STORE_ID); + + /** @var Product $product*/ + $product = $this->productRepository->get('product1'); + + $productFilter = [ + UrlRewrite::ENTITY_TYPE => 'product', + ]; + + //Product in 1st website. Should result in being in 1st and 4th stores. + $expected = [ + [ + 'request_path' => "product-1.html", + 'target_path' => "catalog/product/view/id/" . $product->getId(), + 'is_auto_generated' => 1, + 'redirect_type' => 0, + 'store_id' => $testStore1->getId(), + ], + [ + 'request_path' => "product-1.html", + 'target_path' => "catalog/product/view/id/" . $product->getId(), + 'is_auto_generated' => 1, + 'redirect_type' => 0, + 'store_id' => $testStore4->getId(), + ] + ]; + $actual = $this->getActualResults($productFilter); + foreach ($expected as $row) { + $this->assertContains($row, $actual); + } + + //Set product to be not visible at global scope + $product->setVisibility(Visibility::VISIBILITY_NOT_VISIBLE); + $this->productRepository->save($product); + $expected = []; + $actual = $this->getActualResults($productFilter); + foreach ($expected as $row) { + $this->assertContains($row, $actual); + } + + //Add product to websites corresponding to all 4 stores. + //Rewrites should not be present as the product is hidden + //at the global scope. + $product->setWebsiteIds( + array_unique( + [ + $testStore1->getWebsiteId(), + $testStore2->getWebsiteId(), + $testStore3->getWebsiteId(), + $testStore4->getWebsiteId() + ] + ) + ); + $this->productRepository->save($product); + $expected = []; + $actual = $this->getActualResults($productFilter); + foreach ($expected as $row) { + $this->assertContains($row, $actual); + } + + //Set product to be visible at global scope + $product->setVisibility(Visibility::VISIBILITY_BOTH); + $this->productRepository->save($product); + $expected = [ + [ + 'request_path' => "product-1.html", + 'target_path' => "catalog/product/view/id/" . $product->getId(), + 'is_auto_generated' => 1, + 'redirect_type' => 0, + 'store_id' => $testStore1->getId(), + ], + [ + 'request_path' => "product-1.html", + 'target_path' => "catalog/product/view/id/" . $product->getId(), + 'is_auto_generated' => 1, + 'redirect_type' => 0, + 'store_id' => $testStore2->getId(), + ], + [ + 'request_path' => "product-1.html", + 'target_path' => "catalog/product/view/id/" . $product->getId(), + 'is_auto_generated' => 1, + 'redirect_type' => 0, + 'store_id' => $testStore3->getId(), + ], + [ + 'request_path' => "product-1.html", + 'target_path' => "catalog/product/view/id/" . $product->getId(), + 'is_auto_generated' => 1, + 'redirect_type' => 0, + 'store_id' => $testStore4->getId(), + ] + ]; + $actual = $this->getActualResults($productFilter); + foreach ($expected as $row) { + $this->assertContains($row, $actual); + } + } + + /** + * @magentoDataFixture Magento/Store/_files/second_website_with_two_stores.php + * @magentoDataFixture Magento/CatalogUrlRewrite/_files/product_rewrite_multistore.php + * @magentoAppIsolation enabled + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) + */ + public function testChangeVisibilityLocalScope() + { + $testStore1 = $this->storeManager->getStore('default'); + $testStore2 = $this->storeManager->getStore('fixture_second_store'); + $testStore3 = $this->storeManager->getStore('fixture_third_store'); + $testStore4 = $this->storeManager->getStore('test'); + + $this->storeManager->setCurrentStore(Store::DEFAULT_STORE_ID); + + /** @var Product $product*/ + $product = $this->productRepository->get('product1'); + + $productFilter = [ + UrlRewrite::ENTITY_TYPE => 'product', + ]; + + //Product in 1st website. Should result in being in 1st and 4th stores. + $expected = [ + [ + 'request_path' => "product-1.html", + 'target_path' => "catalog/product/view/id/" . $product->getId(), + 'is_auto_generated' => 1, + 'redirect_type' => 0, + 'store_id' => $testStore1->getId(), + ], + [ + 'request_path' => "product-1.html", + 'target_path' => "catalog/product/view/id/" . $product->getId(), + 'is_auto_generated' => 1, + 'redirect_type' => 0, + 'store_id' => $testStore4->getId(), + ] + ]; + $actual = $this->getActualResults($productFilter); + foreach ($expected as $row) { + $this->assertContains($row, $actual); + } + + //Set product to be not visible at store 4 scope + //Rewrite should only be present for store 1 + $this->storeManager->setCurrentStore($testStore4); + $product = $this->productRepository->get('product1'); + $product->setVisibility(Visibility::VISIBILITY_NOT_VISIBLE); + $this->productRepository->save($product); + $expected = [ + [ + 'request_path' => "product-1.html", + 'target_path' => "catalog/product/view/id/" . $product->getId(), + 'is_auto_generated' => 1, + 'redirect_type' => 0, + 'store_id' => $testStore1->getId(), + ] + ]; + $actual = $this->getActualResults($productFilter); + foreach ($expected as $row) { + $this->assertContains($row, $actual); + } + + //Add product to websites corresponding to all 4 stores. + //Rewrites should be present for stores 1,2 and 3. + //No rewrites should be present for store 4 as that is not visible + //at local scope. + $this->storeManager->setCurrentStore(Store::DEFAULT_STORE_ID); + $product = $this->productRepository->get('product1'); + $product->setWebsiteIds( + array_unique( + [ + $testStore1->getWebsiteId(), + $testStore2->getWebsiteId(), + $testStore3->getWebsiteId(), + $testStore4->getWebsiteId() + ] + ) + ); + $this->productRepository->save($product); + $expected = [ + [ + 'request_path' => "product-1.html", + 'target_path' => "catalog/product/view/id/" . $product->getId(), + 'is_auto_generated' => 1, + 'redirect_type' => 0, + 'store_id' => $testStore1->getId(), ], [ 'request_path' => "product-1.html", 'target_path' => "catalog/product/view/id/" . $product->getId(), 'is_auto_generated' => 1, 'redirect_type' => 0, - 'store_id' => $testStore->getId(), + 'store_id' => $testStore2->getId(), ], + [ + 'request_path' => "product-1.html", + 'target_path' => "catalog/product/view/id/" . $product->getId(), + 'is_auto_generated' => 1, + 'redirect_type' => 0, + 'store_id' => $testStore3->getId(), + ] ]; + $actual = $this->getActualResults($productFilter); + foreach ($expected as $row) { + $this->assertContains($row, $actual); + } + //Set product to be visible at store 4 scope only + $this->storeManager->setCurrentStore($testStore4); + $product = $this->productRepository->get('product1'); + $product->setVisibility(Visibility::VISIBILITY_BOTH); + $this->productRepository->save($product); + $expected = [ + [ + 'request_path' => "product-1.html", + 'target_path' => "catalog/product/view/id/" . $product->getId(), + 'is_auto_generated' => 1, + 'redirect_type' => 0, + 'store_id' => $testStore1->getId(), + ], + [ + 'request_path' => "product-1.html", + 'target_path' => "catalog/product/view/id/" . $product->getId(), + 'is_auto_generated' => 1, + 'redirect_type' => 0, + 'store_id' => $testStore2->getId(), + ], + [ + 'request_path' => "product-1.html", + 'target_path' => "catalog/product/view/id/" . $product->getId(), + 'is_auto_generated' => 1, + 'redirect_type' => 0, + 'store_id' => $testStore3->getId(), + ], + [ + 'request_path' => "product-1.html", + 'target_path' => "catalog/product/view/id/" . $product->getId(), + 'is_auto_generated' => 1, + 'redirect_type' => 0, + 'store_id' => $testStore4->getId(), + ] + ]; $actual = $this->getActualResults($productFilter); foreach ($expected as $row) { $this->assertContains($row, $actual); From 790546250726fe83364a391cf544ae9b89a67742 Mon Sep 17 00:00:00 2001 From: Graham Wharton Date: Wed, 4 Mar 2020 13:10:38 +0000 Subject: [PATCH 03/11] Added CatalogUrlRewrite MFTF tests to urlRewrite test group --- .../Test/AdminCategoryWithRestrictedUrlKeyNotCreatedTest.xml | 1 + .../Test/Mftf/Test/AdminUrlForProductRewrittenCorrectlyTest.xml | 1 + .../Mftf/Test/RewriteStoreLevelUrlKeyOfChildCategoryTest.xml | 1 + .../Test/StorefrontCategoryAccessibleWhenSuffixIsNullTest.xml | 1 + 4 files changed, 4 insertions(+) diff --git a/app/code/Magento/CatalogUrlRewrite/Test/Mftf/Test/AdminCategoryWithRestrictedUrlKeyNotCreatedTest.xml b/app/code/Magento/CatalogUrlRewrite/Test/Mftf/Test/AdminCategoryWithRestrictedUrlKeyNotCreatedTest.xml index c3c7e3c3281f4..d51c9884ab3fa 100644 --- a/app/code/Magento/CatalogUrlRewrite/Test/Mftf/Test/AdminCategoryWithRestrictedUrlKeyNotCreatedTest.xml +++ b/app/code/Magento/CatalogUrlRewrite/Test/Mftf/Test/AdminCategoryWithRestrictedUrlKeyNotCreatedTest.xml @@ -17,6 +17,7 @@ + diff --git a/app/code/Magento/CatalogUrlRewrite/Test/Mftf/Test/AdminUrlForProductRewrittenCorrectlyTest.xml b/app/code/Magento/CatalogUrlRewrite/Test/Mftf/Test/AdminUrlForProductRewrittenCorrectlyTest.xml index cc5f09faca57b..211a1b107b596 100644 --- a/app/code/Magento/CatalogUrlRewrite/Test/Mftf/Test/AdminUrlForProductRewrittenCorrectlyTest.xml +++ b/app/code/Magento/CatalogUrlRewrite/Test/Mftf/Test/AdminUrlForProductRewrittenCorrectlyTest.xml @@ -17,6 +17,7 @@ + diff --git a/app/code/Magento/CatalogUrlRewrite/Test/Mftf/Test/RewriteStoreLevelUrlKeyOfChildCategoryTest.xml b/app/code/Magento/CatalogUrlRewrite/Test/Mftf/Test/RewriteStoreLevelUrlKeyOfChildCategoryTest.xml index c3a358bbbd292..a561b916d4f10 100644 --- a/app/code/Magento/CatalogUrlRewrite/Test/Mftf/Test/RewriteStoreLevelUrlKeyOfChildCategoryTest.xml +++ b/app/code/Magento/CatalogUrlRewrite/Test/Mftf/Test/RewriteStoreLevelUrlKeyOfChildCategoryTest.xml @@ -15,6 +15,7 @@ + diff --git a/app/code/Magento/CatalogUrlRewrite/Test/Mftf/Test/StorefrontCategoryAccessibleWhenSuffixIsNullTest.xml b/app/code/Magento/CatalogUrlRewrite/Test/Mftf/Test/StorefrontCategoryAccessibleWhenSuffixIsNullTest.xml index 6674c55064169..8bef7a81ebd18 100644 --- a/app/code/Magento/CatalogUrlRewrite/Test/Mftf/Test/StorefrontCategoryAccessibleWhenSuffixIsNullTest.xml +++ b/app/code/Magento/CatalogUrlRewrite/Test/Mftf/Test/StorefrontCategoryAccessibleWhenSuffixIsNullTest.xml @@ -15,6 +15,7 @@ + From 98b745f12bb88e712c703e5abcc390a4af26d6fb Mon Sep 17 00:00:00 2001 From: Graham Wharton Date: Wed, 4 Mar 2020 14:02:59 +0000 Subject: [PATCH 04/11] Resolved test failures --- .../Plugin/ProductProcessUrlRewriteRemovingPlugin.php | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/app/code/Magento/CatalogUrlRewrite/Plugin/ProductProcessUrlRewriteRemovingPlugin.php b/app/code/Magento/CatalogUrlRewrite/Plugin/ProductProcessUrlRewriteRemovingPlugin.php index 1bec43c4fd75b..d536b61bd4a79 100644 --- a/app/code/Magento/CatalogUrlRewrite/Plugin/ProductProcessUrlRewriteRemovingPlugin.php +++ b/app/code/Magento/CatalogUrlRewrite/Plugin/ProductProcessUrlRewriteRemovingPlugin.php @@ -28,7 +28,7 @@ class ProductProcessUrlRewriteRemovingPlugin /** * @var ProductRepositoryInterface $productRepository */ - protected $productRepository; + private $productRepository; /** * @var StoreWebsiteRelationInterface $storeWebsiteRelation @@ -38,12 +38,12 @@ class ProductProcessUrlRewriteRemovingPlugin /** * @var UrlPersistInterface $urlPersist */ - protected $urlPersist; + private $urlPersist; /** * @var ProductUrlRewriteGenerator $productUrlRewriteGenerator */ - protected $productUrlRewriteGenerator; + private $productUrlRewriteGenerator; /** * @var DbStorage $dbStorage @@ -72,13 +72,16 @@ public function __construct( } /** + * Function afterUpdateWebsites + * * @param Action $subject - * @param null $result + * @param $result * @param array $productIds * @param array $websiteIds * @param string $type * @return void * @SuppressWarnings(PHPMD.UnusedFormalParameter) + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ public function afterUpdateWebsites( Action $subject, From 8f4e576d2a36e76e1ef5fddfda9c70770b77bb0d Mon Sep 17 00:00:00 2001 From: Graham Wharton Date: Wed, 4 Mar 2020 14:43:27 +0000 Subject: [PATCH 05/11] Fixed test failure --- .../Plugin/ProductProcessUrlRewriteRemovingPlugin.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/CatalogUrlRewrite/Plugin/ProductProcessUrlRewriteRemovingPlugin.php b/app/code/Magento/CatalogUrlRewrite/Plugin/ProductProcessUrlRewriteRemovingPlugin.php index d536b61bd4a79..20d376bd32825 100644 --- a/app/code/Magento/CatalogUrlRewrite/Plugin/ProductProcessUrlRewriteRemovingPlugin.php +++ b/app/code/Magento/CatalogUrlRewrite/Plugin/ProductProcessUrlRewriteRemovingPlugin.php @@ -75,7 +75,7 @@ public function __construct( * Function afterUpdateWebsites * * @param Action $subject - * @param $result + * @param void $result * @param array $productIds * @param array $websiteIds * @param string $type From 385df13013e83d0c4064c678d99b2607dc838186 Mon Sep 17 00:00:00 2001 From: Graham Wharton Date: Thu, 5 Mar 2020 11:21:31 +0000 Subject: [PATCH 06/11] Moved DeleteEntitiesFromStores into own class. --- ...ProductProcessUrlRewriteSavingObserver.php | 25 ++++--- ...ProductProcessUrlRewriteRemovingPlugin.php | 16 ++--- .../Storage/DeleteEntitiesFromStores.php | 68 +++++++++++++++++++ 3 files changed, 88 insertions(+), 21 deletions(-) create mode 100644 app/code/Magento/UrlRewrite/Model/Storage/DeleteEntitiesFromStores.php diff --git a/app/code/Magento/CatalogUrlRewrite/Observer/ProductProcessUrlRewriteSavingObserver.php b/app/code/Magento/CatalogUrlRewrite/Observer/ProductProcessUrlRewriteSavingObserver.php index 7cb916b6a6dd8..3671a25a2d9c0 100644 --- a/app/code/Magento/CatalogUrlRewrite/Observer/ProductProcessUrlRewriteSavingObserver.php +++ b/app/code/Magento/CatalogUrlRewrite/Observer/ProductProcessUrlRewriteSavingObserver.php @@ -13,12 +13,11 @@ use Magento\Framework\Event\Observer; use Magento\Framework\Exception\NoSuchEntityException; use Magento\UrlRewrite\Model\Exception\UrlAlreadyExistsException; +use Magento\UrlRewrite\Model\Storage\DeleteEntitiesFromStores; use Magento\UrlRewrite\Model\UrlPersistInterface; use Magento\Framework\Event\ObserverInterface; use Magento\Store\Model\StoreManagerInterface; use Magento\Store\Api\StoreWebsiteRelationInterface; -use Magento\UrlRewrite\Model\Storage\DbStorage; -use Magento\Store\Model\Store; /** * Class ProductProcessUrlRewriteSavingObserver @@ -30,17 +29,17 @@ class ProductProcessUrlRewriteSavingObserver implements ObserverInterface { /** - * @var ProductUrlRewriteGenerator + * @var ProductUrlRewriteGenerator $productUrlRewriteGenerator */ private $productUrlRewriteGenerator; /** - * @var UrlPersistInterface + * @var UrlPersistInterface $urlPersist */ private $urlPersist; /** - * @var ProductUrlPathGenerator + * @var ProductUrlPathGenerator $productUrlPathGenerator */ private $productUrlPathGenerator; @@ -50,7 +49,7 @@ class ProductProcessUrlRewriteSavingObserver implements ObserverInterface private $storeManager; /** - * @var StoreWebsiteRelationInterface + * @var StoreWebsiteRelationInterface $storeWebsiteRelation */ private $storeWebsiteRelation; @@ -60,14 +59,14 @@ class ProductProcessUrlRewriteSavingObserver implements ObserverInterface private $productRepository; /** - * @var ProductScopeRewriteGenerator + * @var ProductScopeRewriteGenerator $productScopeRewriteGenerator */ private $productScopeRewriteGenerator; /** - * @var DbStorage + * @var DeleteEntitiesFromStores $deleteEntitiesFromStores */ - private $dbStorage; + private $deleteEntitiesFromStores; /** * @param ProductUrlRewriteGenerator $productUrlRewriteGenerator @@ -77,7 +76,7 @@ class ProductProcessUrlRewriteSavingObserver implements ObserverInterface * @param StoreWebsiteRelationInterface $storeWebsiteRelation * @param ProductRepository $productRepository * @param ProductScopeRewriteGenerator $productScopeRewriteGenerator - * @param DbStorage $dbStorage + * @param DeleteEntitiesFromStores $deleteEntitiesFromStores */ public function __construct( ProductUrlRewriteGenerator $productUrlRewriteGenerator, @@ -87,7 +86,7 @@ public function __construct( StoreWebsiteRelationInterface $storeWebsiteRelation, ProductRepository $productRepository, ProductScopeRewriteGenerator $productScopeRewriteGenerator, - DbStorage $dbStorage + DeleteEntitiesFromStores $deleteEntitiesFromStores ) { $this->productUrlRewriteGenerator = $productUrlRewriteGenerator; $this->urlPersist = $urlPersist; @@ -96,7 +95,7 @@ public function __construct( $this->storeWebsiteRelation = $storeWebsiteRelation; $this->productRepository = $productRepository; $this->productScopeRewriteGenerator = $productScopeRewriteGenerator; - $this->dbStorage = $dbStorage; + $this->deleteEntitiesFromStores = $deleteEntitiesFromStores; } /** @@ -152,7 +151,7 @@ public function execute(Observer $observer) } } if (count($storeIdsToRemove)) { - $this->dbStorage->deleteEntitiesFromStores( + $this->deleteEntitiesFromStores->execute( $storeIdsToRemove, [$product->getId()], ProductUrlRewriteGenerator::ENTITY_TYPE diff --git a/app/code/Magento/CatalogUrlRewrite/Plugin/ProductProcessUrlRewriteRemovingPlugin.php b/app/code/Magento/CatalogUrlRewrite/Plugin/ProductProcessUrlRewriteRemovingPlugin.php index 20d376bd32825..3b4929c25a1cd 100644 --- a/app/code/Magento/CatalogUrlRewrite/Plugin/ProductProcessUrlRewriteRemovingPlugin.php +++ b/app/code/Magento/CatalogUrlRewrite/Plugin/ProductProcessUrlRewriteRemovingPlugin.php @@ -13,7 +13,7 @@ use Magento\CatalogUrlRewrite\Model\ProductUrlRewriteGenerator; use Magento\Store\Api\StoreWebsiteRelationInterface; use Magento\Store\Model\Store; -use Magento\UrlRewrite\Model\Storage\DbStorage; +use Magento\UrlRewrite\Model\Storage\DeleteEntitiesFromStores; use Magento\UrlRewrite\Model\UrlPersistInterface; /** @@ -46,29 +46,29 @@ class ProductProcessUrlRewriteRemovingPlugin private $productUrlRewriteGenerator; /** - * @var DbStorage $dbStorage + * @var DeleteEntitiesFromStores $deleteEntitiesFromStores */ - private $dbStorage; + private $deleteEntitiesFromStores; /** * @param ProductRepositoryInterface $productRepository * @param StoreWebsiteRelationInterface $storeWebsiteRelation * @param UrlPersistInterface $urlPersist * @param ProductUrlRewriteGenerator $productUrlRewriteGenerator - * @param DbStorage $dbStorage + * @param DeleteEntitiesFromStores $deleteEntitiesFromStores */ public function __construct( ProductRepositoryInterface $productRepository, StoreWebsiteRelationInterface $storeWebsiteRelation, UrlPersistInterface $urlPersist, ProductUrlRewriteGenerator $productUrlRewriteGenerator, - DbStorage $dbStorage + DeleteEntitiesFromStores $deleteEntitiesFromStores ) { $this->productRepository = $productRepository; $this->storeWebsiteRelation = $storeWebsiteRelation; $this->urlPersist = $urlPersist; $this->productUrlRewriteGenerator = $productUrlRewriteGenerator; - $this->dbStorage = $dbStorage; + $this->deleteEntitiesFromStores = $deleteEntitiesFromStores; } /** @@ -109,14 +109,14 @@ public function afterUpdateWebsites( $storeIdsToRemove = []; // Remove the URLs from websites this product no longer belongs to - if ($type == "remove" && $websiteIds && $productIds) { + if ($type === "remove" && $websiteIds && $productIds) { foreach ($websiteIds as $webId) { foreach ($this->storeWebsiteRelation->getStoreByWebsiteId($webId) as $storeid) { $storeIdsToRemove[] = $storeid; } } if (count($storeIdsToRemove)) { - $this->dbStorage->deleteEntitiesFromStores( + $this->deleteEntitiesFromStores->execute( $storeIdsToRemove, $productIds, ProductUrlRewriteGenerator::ENTITY_TYPE diff --git a/app/code/Magento/UrlRewrite/Model/Storage/DeleteEntitiesFromStores.php b/app/code/Magento/UrlRewrite/Model/Storage/DeleteEntitiesFromStores.php new file mode 100644 index 0000000000000..8f56a382fc888 --- /dev/null +++ b/app/code/Magento/UrlRewrite/Model/Storage/DeleteEntitiesFromStores.php @@ -0,0 +1,68 @@ +connection = $resource->getConnection(); + $this->resource = $resource; + } + + /** + * Function execute + * + * Deletes multiple URL Rewrites from database + * + * @param array $storeIds + * @param array $entityIds + * @param int $entityType + */ + public function execute($storeIds, $entityIds, $entityType) + { + $select = $this->connection->select(); + $select->from($this->resource->getTableName(DbStorage::TABLE_NAME)); + + $select->where( + $this->connection->quoteIdentifier( + UrlRewrite::STORE_ID + ) . ' IN (' . $this->connection->quote($storeIds, 'INTEGER') . ')' . + ' AND ' . $this->connection->quoteIdentifier( + UrlRewrite::ENTITY_ID + ) . ' IN (' . $this->connection->quote($entityIds, 'INTEGER') . ')' . + ' AND ' . $this->connection->quoteIdentifier( + UrlRewrite::ENTITY_TYPE + ) . ' = ' . $this->connection->quote($entityType) + ); + $select = $select->deleteFromSelect($this->resource->getTableName(DbStorage::TABLE_NAME)); + $this->connection->query($select); + } +} From 9e4f01d84fd27605953d8633680d00fbe8590d35 Mon Sep 17 00:00:00 2001 From: Graham Wharton Date: Thu, 5 Mar 2020 11:24:38 +0000 Subject: [PATCH 07/11] Removed replaced function from DbStorage, now implemented in own class. --- .../UrlRewrite/Model/Storage/DbStorage.php | 29 ------------------- 1 file changed, 29 deletions(-) diff --git a/app/code/Magento/UrlRewrite/Model/Storage/DbStorage.php b/app/code/Magento/UrlRewrite/Model/Storage/DbStorage.php index a5d23f99bc0cd..f0e94e8379ad2 100644 --- a/app/code/Magento/UrlRewrite/Model/Storage/DbStorage.php +++ b/app/code/Magento/UrlRewrite/Model/Storage/DbStorage.php @@ -375,33 +375,4 @@ public function deleteByData(array $data) $this->prepareSelect($data)->deleteFromSelect($this->resource->getTableName(self::TABLE_NAME)) ); } - - /** - * Function deleteEntitiesFromStores - * - * Deletes multiple URL Rewrites from database - * - * @param array $store_ids - * @param array $entity_ids - * @param int $entity_type - */ - public function deleteEntitiesFromStores($store_ids, $entity_ids, $entity_type) - { - $select = $this->connection->select(); - $select->from($this->resource->getTableName(self::TABLE_NAME)); - - $select->where( - $this->connection->quoteIdentifier( - UrlRewrite::STORE_ID - ) . ' IN (' . $this->connection->quote($store_ids, 'INTEGER') . ')' . - ' AND ' . $this->connection->quoteIdentifier( - UrlRewrite::ENTITY_ID - ) . ' IN (' . $this->connection->quote($entity_ids, 'INTEGER') . ')' . - ' AND ' . $this->connection->quoteIdentifier( - UrlRewrite::ENTITY_TYPE - ) . ' = ' . $this->connection->quote($entity_type) - ); - $select = $select->deleteFromSelect($this->resource->getTableName(self::TABLE_NAME)); - $this->connection->query($select); - } } From a4700f56d0d9eee961f2cab4404af7295e5db6c7 Mon Sep 17 00:00:00 2001 From: Graham Wharton Date: Thu, 5 Mar 2020 11:36:39 +0000 Subject: [PATCH 08/11] Updated Unit Tests for new Class --- ...oductProcessUrlRewriteSavingObserverTest.php | 17 ++++++++++------- ...oductProcessUrlRewriteRemovingPluginTest.php | 17 ++++++++++------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/ProductProcessUrlRewriteSavingObserverTest.php b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/ProductProcessUrlRewriteSavingObserverTest.php index df1effb2f2eb8..e561c26f98ee6 100644 --- a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/ProductProcessUrlRewriteSavingObserverTest.php +++ b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/ProductProcessUrlRewriteSavingObserverTest.php @@ -12,7 +12,7 @@ use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; use Magento\Store\Api\StoreWebsiteRelationInterface; use Magento\Store\Model\Store; -use Magento\UrlRewrite\Model\Storage\DbStorage; +use Magento\UrlRewrite\Model\Storage\DeleteEntitiesFromStores; use Magento\UrlRewrite\Model\UrlPersistInterface; use Magento\Catalog\Model\Product; use Magento\Framework\Event; @@ -116,9 +116,9 @@ class ProductProcessUrlRewriteSavingObserverTest extends TestCase private $productRepository; /** - * @var DbStorage|MockObject + * @var DeleteEntitiesFromStores|MockObject */ - private $dbStorage; + private $deleteEntitiesFromStores; /** * Set up @@ -165,7 +165,10 @@ protected function setUp() [1, false, 2, true, $this->product2], [1, false, 5, true, $this->product5] ])); - $this->dbStorage = $this->createPartialMock(DbStorage::class, ['deleteEntitiesFromStores']); + $this->deleteEntitiesFromStores = $this->createPartialMock( + DeleteEntitiesFromStores::class, + ['execute'] + ); $this->event = $this->createPartialMock(Event::class, ['getProduct']); $this->event->expects($this->any())->method('getProduct')->willReturn($this->product); $this->observer = $this->createPartialMock(Observer::class, ['getEvent']); @@ -216,7 +219,7 @@ protected function setUp() 'storeManager' => $this->storeManager, 'storeWebsiteRelation' => $this->storeWebsiteRelation, 'productRepository' => $this->productRepository, - 'dbStorage' => $this->dbStorage, + 'deleteEntitiesFromStores' => $this->deleteEntitiesFromStores, 'productScopeRewriteGenerator' => $this->productScopeRewriteGenerator ] ); @@ -387,8 +390,8 @@ public function testExecuteUrlKey( ->method('replace') ->with([1 => 'rewrite']); - $this->dbStorage->expects($this->any()) - ->method('deleteEntitiesFromStores') + $this->deleteEntitiesFromStores->expects($this->any()) + ->method('execute') ->with( $expectedRemoves, [1], diff --git a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Plugin/ProductProcessUrlRewriteRemovingPluginTest.php b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Plugin/ProductProcessUrlRewriteRemovingPluginTest.php index 1e9d6a911fe14..16375da5fdf72 100644 --- a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Plugin/ProductProcessUrlRewriteRemovingPluginTest.php +++ b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Plugin/ProductProcessUrlRewriteRemovingPluginTest.php @@ -8,7 +8,7 @@ use Magento\Catalog\Model\Product\Action; use Magento\CatalogUrlRewrite\Model\ProductUrlRewriteGenerator; use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; -use Magento\UrlRewrite\Model\Storage\DbStorage; +use Magento\UrlRewrite\Model\Storage\DeleteEntitiesFromStores; use Magento\UrlRewrite\Model\UrlPersistInterface; use Magento\Store\Model\StoreManagerInterface; use Magento\Store\Model\Website; @@ -97,9 +97,9 @@ class ProductProcessUrlRewriteRemovingPluginTest extends TestCase private $storeWebsiteRelation; /** - * @var DbStorage|MockObject + * @var DeleteEntitiesFromStores|MockObject */ - private $dbStorage; + private $deleteEntitiesFromStores; /** * Set up @@ -153,7 +153,10 @@ protected function setUp() [3, false, 0, true, $this->product3] ])); - $this->dbStorage = $this->createPartialMock(DbStorage::class, ['deleteEntitiesFromStores']); + $this->deleteEntitiesFromStores = $this->createPartialMock( + DeleteEntitiesFromStores::class, + ['execute'] + ); $this->subject = $this->createMock( Action::class @@ -167,7 +170,7 @@ protected function setUp() 'storeWebsiteRelation' => $this->storeWebsiteRelation, 'urlPersist' => $this->urlPersist, 'productUrlRewriteGenerator' => $this->productUrlRewriteGenerator, - 'dbStorage' => $this->dbStorage + 'deleteEntitiesFromStores' => $this->deleteEntitiesFromStores ] ); @@ -320,8 +323,8 @@ public function testAfterUpdateWebsites( ->method('replace') ->with($rewrites); - $this->dbStorage->expects($this->exactly($expectedDeleteCount)) - ->method('deleteEntitiesFromStores') + $this->deleteEntitiesFromStores->expects($this->exactly($expectedDeleteCount)) + ->method('execute') ->with( $expectedStoreRemovals, $productids, From c340802bb4acd0e0dd1868a56a0f31f51f213fc8 Mon Sep 17 00:00:00 2001 From: Graham Wharton Date: Fri, 6 Mar 2020 14:56:43 +0000 Subject: [PATCH 09/11] Optimised code. Processed review comments. --- ...ProductProcessUrlRewriteSavingObserver.php | 80 ++-- ...ProductProcessUrlRewriteRemovingPlugin.php | 13 +- ...uctProcessUrlRewriteSavingObserverTest.php | 408 ++++++++++++------ .../Storage/DeleteEntitiesFromStores.php | 4 +- ...uctProcessUrlRewriteSavingObserverTest.php | 9 +- 5 files changed, 323 insertions(+), 191 deletions(-) diff --git a/app/code/Magento/CatalogUrlRewrite/Observer/ProductProcessUrlRewriteSavingObserver.php b/app/code/Magento/CatalogUrlRewrite/Observer/ProductProcessUrlRewriteSavingObserver.php index 3671a25a2d9c0..fddc0e49c6e2a 100644 --- a/app/code/Magento/CatalogUrlRewrite/Observer/ProductProcessUrlRewriteSavingObserver.php +++ b/app/code/Magento/CatalogUrlRewrite/Observer/ProductProcessUrlRewriteSavingObserver.php @@ -5,19 +5,19 @@ */ namespace Magento\CatalogUrlRewrite\Observer; +use Magento\Catalog\Api\Data\ProductInterface; use Magento\Catalog\Model\Product; -use Magento\Catalog\Model\ProductRepository; +use Magento\Catalog\Model\Product\Visibility; +use Magento\Catalog\Model\ResourceModel\Product\Collection; use Magento\CatalogUrlRewrite\Model\ProductScopeRewriteGenerator; use Magento\CatalogUrlRewrite\Model\ProductUrlPathGenerator; use Magento\CatalogUrlRewrite\Model\ProductUrlRewriteGenerator; use Magento\Framework\Event\Observer; -use Magento\Framework\Exception\NoSuchEntityException; use Magento\UrlRewrite\Model\Exception\UrlAlreadyExistsException; use Magento\UrlRewrite\Model\Storage\DeleteEntitiesFromStores; use Magento\UrlRewrite\Model\UrlPersistInterface; use Magento\Framework\Event\ObserverInterface; use Magento\Store\Model\StoreManagerInterface; -use Magento\Store\Api\StoreWebsiteRelationInterface; /** * Class ProductProcessUrlRewriteSavingObserver @@ -29,73 +29,65 @@ class ProductProcessUrlRewriteSavingObserver implements ObserverInterface { /** - * @var ProductUrlRewriteGenerator $productUrlRewriteGenerator + * @var ProductUrlRewriteGenerator */ private $productUrlRewriteGenerator; /** - * @var UrlPersistInterface $urlPersist + * @var UrlPersistInterface */ private $urlPersist; /** - * @var ProductUrlPathGenerator $productUrlPathGenerator + * @var ProductUrlPathGenerator */ private $productUrlPathGenerator; /** - * @var StoreManagerInterface $storeManager + * @var StoreManagerInterface */ private $storeManager; /** - * @var StoreWebsiteRelationInterface $storeWebsiteRelation + * @var ProductScopeRewriteGenerator */ - private $storeWebsiteRelation; - - /** - * @var ProductRepository $productRepository - */ - private $productRepository; + private $productScopeRewriteGenerator; /** - * @var ProductScopeRewriteGenerator $productScopeRewriteGenerator + * @var DeleteEntitiesFromStores */ - private $productScopeRewriteGenerator; + private $deleteEntitiesFromStores; /** - * @var DeleteEntitiesFromStores $deleteEntitiesFromStores + * @var Collection */ - private $deleteEntitiesFromStores; + private $productCollection; /** * @param ProductUrlRewriteGenerator $productUrlRewriteGenerator * @param UrlPersistInterface $urlPersist * @param ProductUrlPathGenerator $productUrlPathGenerator * @param StoreManagerInterface $storeManager - * @param StoreWebsiteRelationInterface $storeWebsiteRelation - * @param ProductRepository $productRepository * @param ProductScopeRewriteGenerator $productScopeRewriteGenerator * @param DeleteEntitiesFromStores $deleteEntitiesFromStores + * @param Collection $productCollection */ public function __construct( ProductUrlRewriteGenerator $productUrlRewriteGenerator, UrlPersistInterface $urlPersist, ProductUrlPathGenerator $productUrlPathGenerator, StoreManagerInterface $storeManager, - StoreWebsiteRelationInterface $storeWebsiteRelation, - ProductRepository $productRepository, ProductScopeRewriteGenerator $productScopeRewriteGenerator, - DeleteEntitiesFromStores $deleteEntitiesFromStores + DeleteEntitiesFromStores $deleteEntitiesFromStores, + Collection $productCollection ) { $this->productUrlRewriteGenerator = $productUrlRewriteGenerator; $this->urlPersist = $urlPersist; $this->productUrlPathGenerator = $productUrlPathGenerator; $this->storeManager = $storeManager; - $this->storeWebsiteRelation = $storeWebsiteRelation; - $this->productRepository = $productRepository; $this->productScopeRewriteGenerator = $productScopeRewriteGenerator; $this->deleteEntitiesFromStores = $deleteEntitiesFromStores; + $this->productCollection = $productCollection; } /** @@ -104,8 +96,6 @@ public function __construct( * @param Observer $observer * @return void * @throws UrlAlreadyExistsException - * @throws NoSuchEntityException - * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ public function execute(Observer $observer) { @@ -125,29 +115,31 @@ public function execute(Observer $observer) } $storeIdsToRemove = []; + $productWebsiteMap = array_flip($product->getWebsiteIds()); + $storeVisibilities = $this->productCollection->getAllAttributeValues(ProductInterface::VISIBILITY); if ($this->productScopeRewriteGenerator->isGlobalScope($product->getStoreId())) { //Remove any rewrite URLs for websites the product is not in, or is not visible in. Global Scope. - foreach ($this->storeManager->getWebsites() as $website) { - $websiteId = $website->getWebsiteId(); - foreach ($this->storeWebsiteRelation->getStoreByWebsiteId($websiteId) as $storeid) { - //Load the product for the store we are processing so we can see if it is visible - $storeProduct = $this->productRepository->getById( - $product->getId(), - false, - $storeid, - true - ); - if (!$storeProduct->isVisibleInSiteVisibility() || - !in_array($websiteId, $product->getWebsiteIds())) { - $storeIdsToRemove[] = $storeid; - }; + foreach ($this->storeManager->getStores() as $store) { + $websiteId = $store->getWebsiteId(); + $storeId = $store->getStoreId(); + if (!isset($productWebsiteMap[$websiteId])) { + $storeIdsToRemove[] = $storeId; + continue; + } + //Check the visibility of the product in each store. + if (isset($storeVisibilities[$product->getId()][$storeId]) + && ($storeVisibilities[$product->getId()][$storeId] === Visibility::VISIBILITY_NOT_VISIBLE)) { + $storeIdsToRemove[] = $storeId; } } } else { //Only remove rewrite for current scope - if (!$product->isVisibleInSiteVisibility() || - !in_array($product->getStoreId(), $product->getStoreIds())) { - $storeIdsToRemove[] = $product->getStoreId(); + $websiteId = $product->getStore()->getWebsiteId(); + $storeId = $product->getStoreId(); + if (!isset($productWebsiteMap[$websiteId]) || + (isset($storeVisibilities[$product->getId()][$storeId]) + && ($storeVisibilities[$product->getId()][$storeId] === Visibility::VISIBILITY_NOT_VISIBLE))) { + $storeIdsToRemove[] = $storeId; } } if (count($storeIdsToRemove)) { diff --git a/app/code/Magento/CatalogUrlRewrite/Plugin/ProductProcessUrlRewriteRemovingPlugin.php b/app/code/Magento/CatalogUrlRewrite/Plugin/ProductProcessUrlRewriteRemovingPlugin.php index 3b4929c25a1cd..629649897b9de 100644 --- a/app/code/Magento/CatalogUrlRewrite/Plugin/ProductProcessUrlRewriteRemovingPlugin.php +++ b/app/code/Magento/CatalogUrlRewrite/Plugin/ProductProcessUrlRewriteRemovingPlugin.php @@ -26,27 +26,27 @@ class ProductProcessUrlRewriteRemovingPlugin { /** - * @var ProductRepositoryInterface $productRepository + * @var ProductRepositoryInterface */ private $productRepository; /** - * @var StoreWebsiteRelationInterface $storeWebsiteRelation + * @var StoreWebsiteRelationInterface */ private $storeWebsiteRelation; /** - * @var UrlPersistInterface $urlPersist + * @var UrlPersistInterface */ private $urlPersist; /** - * @var ProductUrlRewriteGenerator $productUrlRewriteGenerator + * @var ProductUrlRewriteGenerator */ private $productUrlRewriteGenerator; /** - * @var DeleteEntitiesFromStores $deleteEntitiesFromStores + * @var DeleteEntitiesFromStores */ private $deleteEntitiesFromStores; @@ -108,7 +108,8 @@ public function afterUpdateWebsites( } $storeIdsToRemove = []; - // Remove the URLs from websites this product no longer belongs to + // Remove the URLs for products in $productIds array + // from all stores that belong to websites in $websiteIds array if ($type === "remove" && $websiteIds && $productIds) { foreach ($websiteIds as $webId) { foreach ($this->storeWebsiteRelation->getStoreByWebsiteId($webId) as $storeid) { diff --git a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/ProductProcessUrlRewriteSavingObserverTest.php b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/ProductProcessUrlRewriteSavingObserverTest.php index e561c26f98ee6..c2dcb1195bbce 100644 --- a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/ProductProcessUrlRewriteSavingObserverTest.php +++ b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/ProductProcessUrlRewriteSavingObserverTest.php @@ -6,12 +6,13 @@ namespace Magento\CatalogUrlRewrite\Test\Unit\Observer; -use Magento\Catalog\Model\ProductRepository; +use Magento\Catalog\Model\ResourceModel\Product\Collection; use Magento\CatalogUrlRewrite\Model\ProductScopeRewriteGenerator; use Magento\CatalogUrlRewrite\Model\ProductUrlRewriteGenerator; use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; use Magento\Store\Api\StoreWebsiteRelationInterface; use Magento\Store\Model\Store; +use Magento\UrlRewrite\Model\Exception\UrlAlreadyExistsException; use Magento\UrlRewrite\Model\Storage\DeleteEntitiesFromStores; use Magento\UrlRewrite\Model\UrlPersistInterface; use Magento\Catalog\Model\Product; @@ -55,21 +56,6 @@ class ProductProcessUrlRewriteSavingObserverTest extends TestCase */ private $product; - /** - * @var Product|MockObject - */ - private $product1; - - /** - * @var Product|MockObject - */ - private $product2; - - /** - * @var Product|MockObject - */ - private $product5; - /** * @var ProductUrlRewriteGenerator|MockObject */ @@ -96,14 +82,14 @@ class ProductProcessUrlRewriteSavingObserverTest extends TestCase private $storeManager; /** - * @var Website|MockObject + * @var array */ - private $website1; + private $websites; /** - * @var Website|MockObject + * @var array */ - private $website2; + private $stores; /** * @var StoreWebsiteRelationInterface|MockObject @@ -111,75 +97,59 @@ class ProductProcessUrlRewriteSavingObserverTest extends TestCase private $storeWebsiteRelation; /** - * @var ProductRepository|MockObject + * @var DeleteEntitiesFromStores|MockObject */ - private $productRepository; + private $deleteEntitiesFromStores; /** - * @var DeleteEntitiesFromStores|MockObject + * @var Collection|MockObject */ - private $deleteEntitiesFromStores; + private $productCollection; /** * Set up + * Website_ID = 0 -> Store_ID = 0 * Website_ID = 1 -> Store_ID = 1 * Website_ID = 2 -> Store_ID = 2 & 5 */ protected function setUp() { + $this->objectManager = new ObjectManager($this); + $this->urlPersist = $this->createMock(UrlPersistInterface::class); - $this->product = $this->createPartialMock( - Product::class, - [ - 'getId', - 'dataHasChangedFor', - 'isVisibleInSiteVisibility', - 'getIsChangedWebsites', - 'getIsChangedCategories', - 'getStoreId', - 'getWebsiteIds' - ] - ); - $this->product1 = $this->createPartialMock( - Product::class, - ['getId', 'isVisibleInSiteVisibility'] - ); - $this->product2 = $this->createPartialMock( - Product::class, - ['getId', 'isVisibleInSiteVisibility'] - ); - $this->product5 = $this->createPartialMock( - Product::class, - ['getId', 'isVisibleInSiteVisibility'] + + $this->websites[0] = $this->initialiseWebsite(0); + $this->websites[1] = $this->initialiseWebsite(1); + $this->websites[2] = $this->initialiseWebsite(2); + + $this->stores[0] = $this->initialiseStore(0, 0); + $this->stores[1] = $this->initialiseStore(1, 1); + $this->stores[2] = $this->initialiseStore(2, 2); + $this->stores[5] = $this->initialiseStore(5, 2); + + $this->product = $this->initialiseProduct($this->stores[0], 0); + + $this->productCollection = $this->createPartialMock(Collection::class, + ['getAllAttributeValues'] ); - $this->productRepository = $this->createPartialMock(ProductRepository::class, ['getById']); - $this->product->expects($this->any())->method('getId')->will($this->returnValue(1)); - $this->product1->expects($this->any())->method('getId')->will($this->returnValue(1)); - $this->product2->expects($this->any())->method('getId')->will($this->returnValue(1)); - $this->product5->expects($this->any())->method('getId')->will($this->returnValue(1)); - $this->productRepository->expects($this->any()) - ->method('getById') - ->will($this->returnValueMap([ - [1, false, 0, true, $this->product], - [1, false, 1, true, $this->product1], - [1, false, 2, true, $this->product2], - [1, false, 5, true, $this->product5] - ])); + $this->deleteEntitiesFromStores = $this->createPartialMock( DeleteEntitiesFromStores::class, ['execute'] ); + $this->event = $this->createPartialMock(Event::class, ['getProduct']); - $this->event->expects($this->any())->method('getProduct')->willReturn($this->product); + $this->event->expects($this->any()) + ->method('getProduct') + ->willReturn($this->product); + $this->observer = $this->createPartialMock(Observer::class, ['getEvent']); $this->observer->expects($this->any())->method('getEvent')->willReturn($this->event); + $this->productUrlRewriteGenerator = $this->createPartialMock( ProductUrlRewriteGenerator::class, ['generate'] ); - $this->productUrlRewriteGenerator->expects($this->any()) - ->method('generate') - ->will($this->returnValue([1 => 'rewrite'])); $this->productScopeRewriteGenerator = $this->createPartialMock( ProductScopeRewriteGenerator::class, ['isGlobalScope'] @@ -193,15 +163,16 @@ protected function setUp() [2, false], [5, false], ])); - $this->objectManager = new ObjectManager($this); - $this->storeManager = $this->createMock(StoreManagerInterface::class); - $this->website1 = $this->createPartialMock(Website::class, ['getWebsiteId']); - $this->website1->expects($this->any())->method('getWebsiteId')->willReturn(1); - $this->website2 = $this->createPartialMock(Website::class, ['getWebsiteId']); - $this->website2->expects($this->any())->method('getWebsiteId')->willReturn(2); + + $this->storeManager = $this->getMockBuilder(StoreManagerInterface::class) + ->disableOriginalConstructor() + ->getMock(); $this->storeManager->expects($this->any()) ->method('getWebsites') - ->will($this->returnValue([$this->website1, $this->website2])); + ->will($this->returnValue([$this->websites[1], $this->websites[2]])); + $this->storeManager->expects($this->any()) + ->method('getStores') + ->will($this->returnValue([$this->stores[1], $this->stores[2], $this->stores[5]])); $this->storeWebsiteRelation = $this->createPartialMock( StoreWebsiteRelationInterface::class, @@ -218,11 +189,64 @@ protected function setUp() 'urlPersist' => $this->urlPersist, 'storeManager' => $this->storeManager, 'storeWebsiteRelation' => $this->storeWebsiteRelation, - 'productRepository' => $this->productRepository, 'deleteEntitiesFromStores' => $this->deleteEntitiesFromStores, - 'productScopeRewriteGenerator' => $this->productScopeRewriteGenerator + 'productScopeRewriteGenerator' => $this->productScopeRewriteGenerator, + 'productCollection' => $this->productCollection + ] + ); + } + + /** + * Initialise product for test + * + * @param $store + * @param $storeId + * @return MockObject + */ + public function initialiseProduct($store, $storeId) + { + $product = $this->createPartialMock( + Product::class, + [ + 'getId', + 'dataHasChangedFor', + 'isVisibleInSiteVisibility', + 'getIsChangedWebsites', + 'getIsChangedCategories', + 'getStoreId', + 'getWebsiteIds', + 'getStore' ] ); + $product->expects($this->any())->method('getId')->will($this->returnValue(1)); + return $product; + } + + /** + * Initialise website for test + * + * @param $websiteId + * @return MockObject + */ + public function initialiseWebsite($websiteId) + { + $website = $this->createPartialMock(Website::class, ['getWebsiteId']); + $website->expects($this->any())->method('getWebsiteId')->willReturn($websiteId); + return $website; + } + + /** + * Initialise store for test + * + * @param $storeId + * @return mixed + */ + public function initialiseStore($storeId, $websiteId) + { + $store = $this->createPartialMock(Store::class, ['getStoreId','getWebsiteId']); + $store->expects($this->any())->method('getStoreId')->willReturn($storeId); + $store->expects($this->any())->method('getWebsiteId')->willReturn($websiteId); + return $store; } /** @@ -233,129 +257,244 @@ protected function setUp() public function urlKeyDataProvider() { return [ - 'url changed' => [ + //url has changed, so we would expect to see a replace issued + //and the urls removed from the stores the product is not in + //i.e stores belonging to website 2 + 'global_scope_url_changed' => [ + 'productScope' => 0, 'isChangedUrlKey' => true, 'isChangedVisibility' => false, 'isChangedWebsites' => false, 'isChangedCategories' => false, - 'visibilityResult' => [ - '0' => true, - '1' => true, - '2' => true, - '5' => true + 'visibility' => [ + 1 => [ + 0 => Product\Visibility::VISIBILITY_BOTH, + 1 => Product\Visibility::VISIBILITY_BOTH, + 2 => Product\Visibility::VISIBILITY_BOTH, + 5 => Product\Visibility::VISIBILITY_BOTH, + ], ], 'productInWebsites' => [1], 'expectedReplaceCount' => 1, 'expectedRemoves' => [2, 5], ], - 'no changes' => [ + //Nothing has changed, so no replaces or removes + 'global_scope_no_changes' => [ + 'productScope' => 0, 'isChangedUrlKey' => false, 'isChangedVisibility' => false, 'isChangedWebsites' => false, 'isChangedCategories' => false, - 'visibilityResult' => [ - '0' => true, - '1' => true, - '2' => true, - '5' => true + 'visibility' => [ + 1 => [ + 0 => Product\Visibility::VISIBILITY_BOTH, + 1 => Product\Visibility::VISIBILITY_BOTH, + 2 => Product\Visibility::VISIBILITY_BOTH, + 5 => Product\Visibility::VISIBILITY_BOTH, + ], ], - 'productInWebsites' => [1, 2], + 'productInWebsites' => [1], 'expectedReplaceCount' => 0, 'expectedRemoves' => [], ], - 'visibility changed' => [ + //Product passed in had global scope set, but the visibility + //at local scope for store 2 is false. Expect to see refresh + //of urls and removal from store 2 + 'global_scope_visibility_changed_local' => [ + 'productScope' => 0, 'isChangedUrlKey' => false, 'isChangedVisibility' => true, 'isChangedWebsites' => false, 'isChangedCategories' => false, - 'visibilityResult' => [ - '0' => true, - '1' => true, - '2' => true, - '5' => true + 'visibility' => [ + 1 => [ + 0 => Product\Visibility::VISIBILITY_BOTH, + 1 => Product\Visibility::VISIBILITY_BOTH, + 2 => Product\Visibility::VISIBILITY_NOT_VISIBLE, + 5 => Product\Visibility::VISIBILITY_BOTH, + ], ], 'productInWebsites' => [1, 2], 'expectedReplaceCount' => 1, - 'expectedRemoves' => [], + 'expectedRemoves' => [2], ], - 'websites changed' => [ + //Product passed in had global scope set, but the visibility + //for all stores is false. Expect to see removal from stores 1,2 and 5 + 'global_scope_visibility_changed_global' => [ + 'productScope' => 0, + 'isChangedUrlKey' => false, + 'isChangedVisibility' => true, + 'isChangedWebsites' => false, + 'isChangedCategories' => false, + 'visibility' => [ + 1 => [ + 0 => Product\Visibility::VISIBILITY_NOT_VISIBLE, + 1 => Product\Visibility::VISIBILITY_NOT_VISIBLE, + 2 => Product\Visibility::VISIBILITY_NOT_VISIBLE, + 5 => Product\Visibility::VISIBILITY_NOT_VISIBLE, + ], + ], + 'productInWebsites' => [1, 2], + 'expectedReplaceCount' => 0, + 'expectedRemoves' => [1, 2, 5], + ], + //Product has changed websites. Now in websites 1 and 2 + //We would expect to see a replace but no removals as the + //product is in all stores + 'global_scope_websites_changed' => [ + 'productScope' => 0, 'isChangedUrlKey' => false, 'isChangedVisibility' => false, 'isChangedWebsites' => true, 'isChangedCategories' => false, - 'visibilityResult' => [ - '0' => true, - '1' => true, - '2' => true, - '5' => true + 'visibility' => [ + 1 => [ + 0 => Product\Visibility::VISIBILITY_BOTH, + 1 => Product\Visibility::VISIBILITY_BOTH, + 2 => Product\Visibility::VISIBILITY_BOTH, + 5 => Product\Visibility::VISIBILITY_BOTH, + ], ], 'productInWebsites' => [1, 2], 'expectedReplaceCount' => 1, 'expectedRemoves' => [], ], - 'categories changed' => [ + //Global scope, all visible, categories changed. + //Expect to see replace and no removals. + 'global_scope_categories_changed' => [ + 'productScope' => 0, 'isChangedUrlKey' => false, 'isChangedVisibility' => false, 'isChangedWebsites' => false, 'isChangedCategories' => true, - 'visibilityResult' => [ - '0' => true, - '1' => true, - '2' => true, - '5' => true + 'visibility' => [ + 1 => [ + 0 => Product\Visibility::VISIBILITY_BOTH, + 1 => Product\Visibility::VISIBILITY_BOTH, + 2 => Product\Visibility::VISIBILITY_BOTH, + 5 => Product\Visibility::VISIBILITY_BOTH, + ], ], 'productInWebsites' => [1, 2], 'expectedReplaceCount' => 1, 'expectedRemoves' => [], ], - 'url changed invisible' => [ + //Global scope, url key has changed but products are + //invisible in all stores, therefore remove any urls if + //they exist. + 'global_scope_url_changed_invisible' => [ + 'productScope' => 0, 'isChangedUrlKey' => true, 'isChangedVisibility' => false, 'isChangedWebsites' => false, 'isChangedCategories' => false, - 'visibilityResult' => [ - '0' => false, - '1' => false, - '2' => false, - '5' => false + 'visibility' => [ + 1 => [ + 0 => Product\Visibility::VISIBILITY_NOT_VISIBLE, + 1 => Product\Visibility::VISIBILITY_NOT_VISIBLE, + 2 => Product\Visibility::VISIBILITY_NOT_VISIBLE, + 5 => Product\Visibility::VISIBILITY_NOT_VISIBLE, + ], ], 'productInWebsites' => [1, 2], 'expectedReplaceCount' => 1, - 'expectedRemoves' => [1,2,5], + 'expectedRemoves' => [1, 2, 5], + ], + //local scope tests should only adjust URLs for local scope + //Even if there are changes to the same product in other stores + //they should be ignored. Here product in store 2 has been set + //visible. Do not expect to see any removals for the other stores. + 'local_scope_visibility_changed_local_1' => [ + 'productScope' => 2, + 'isChangedUrlKey' => false, + 'isChangedVisibility' => true, + 'isChangedWebsites' => false, + 'isChangedCategories' => false, + 'visibility' => [ + 1 => [ + 0 => Product\Visibility::VISIBILITY_NOT_VISIBLE, + 1 => Product\Visibility::VISIBILITY_NOT_VISIBLE, + 2 => Product\Visibility::VISIBILITY_BOTH, + 5 => Product\Visibility::VISIBILITY_NOT_VISIBLE, + ], + ], + 'productInWebsites' => [1, 2], + 'expectedReplaceCount' => 1, + 'expectedRemoves' => [], + ], + //Local scope, so only expecting to operate on store 2. + //Product has been set invisible, removal expected. + 'local_scope_visibility_changed_local_2' => [ + 'productScope' => 2, + 'isChangedUrlKey' => false, + 'isChangedVisibility' => true, + 'isChangedWebsites' => false, + 'isChangedCategories' => false, + 'visibility' => [ + 1 => [ + 0 => Product\Visibility::VISIBILITY_BOTH, + 1 => Product\Visibility::VISIBILITY_BOTH, + 2 => Product\Visibility::VISIBILITY_NOT_VISIBLE, + 5 => Product\Visibility::VISIBILITY_BOTH, + ], + ], + 'productInWebsites' => [1, 2], + 'expectedReplaceCount' => 0, + 'expectedRemoves' => [2], + ], + //Local scope, so only operate on store 5. + //Visibility is false, so see only removal from + //store 5. + 'local_scope_visibility_changed_global' => [ + 'productScope' => 5, + 'isChangedUrlKey' => false, + 'isChangedVisibility' => true, + 'isChangedWebsites' => false, + 'isChangedCategories' => false, + 'visibility' => [ + 1 => [ + 0 => Product\Visibility::VISIBILITY_NOT_VISIBLE, + 1 => Product\Visibility::VISIBILITY_NOT_VISIBLE, + 2 => Product\Visibility::VISIBILITY_NOT_VISIBLE, + 5 => Product\Visibility::VISIBILITY_NOT_VISIBLE, + ], + ], + 'productInWebsites' => [1, 2], + 'expectedReplaceCount' => 0, + 'expectedRemoves' => [5], ], ]; } /** + * @param int $productScope * @param bool $isChangedUrlKey * @param bool $isChangedVisibility * @param bool $isChangedWebsites * @param bool $isChangedCategories - * @param array $visibilityResult + * @param array $visibility * @param int $productInWebsites * @param int $expectedReplaceCount * @param array $expectedRemoves * * @dataProvider urlKeyDataProvider + * @throws UrlAlreadyExistsException */ public function testExecuteUrlKey( + $productScope, $isChangedUrlKey, $isChangedVisibility, $isChangedWebsites, $isChangedCategories, - $visibilityResult, + $visibility, $productInWebsites, $expectedReplaceCount, $expectedRemoves ) { - $this->product->expects($this->any())->method('getStoreId')->will( - $this->returnValue(Store::DEFAULT_STORE_ID) - ); - $this->product->expects($this->any())->method('getWebsiteIds')->will( - $this->returnValue($productInWebsites) - ); - + $this->product->expects($this->any()) + ->method('getWebsiteIds') + ->will($this->returnValue($productInWebsites)); $this->product->expects($this->any()) ->method('dataHasChangedFor') ->will($this->returnValueMap( @@ -364,7 +503,6 @@ public function testExecuteUrlKey( ['url_key', $isChangedUrlKey] ] )); - $this->product->expects($this->any()) ->method('getIsChangedWebsites') ->will($this->returnValue($isChangedWebsites)); @@ -372,23 +510,23 @@ public function testExecuteUrlKey( $this->product->expects($this->any()) ->method('getIsChangedCategories') ->will($this->returnValue($isChangedCategories)); - $this->product->expects($this->any()) - ->method('isVisibleInSiteVisibility') - ->will($this->returnValue($visibilityResult['0'])); - $this->product1->expects($this->any()) - ->method('isVisibleInSiteVisibility') - ->will($this->returnValue($visibilityResult['1'])); - $this->product2->expects($this->any()) - ->method('isVisibleInSiteVisibility') - ->will($this->returnValue($visibilityResult['2'])); - $this->product5->expects($this->any()) - ->method('isVisibleInSiteVisibility') - ->will($this->returnValue($visibilityResult['5'])); + ->method('getStoreId') + ->willReturn($productScope); + $this->product->expects($this->any()) + ->method('getStore') + ->willReturn($this->stores[$productScope]); + $this->productCollection->expects($this->any()) + ->method('getAllAttributeValues') + ->will($this->returnValue($visibility)); + + $this->productUrlRewriteGenerator->expects($this->any()) + ->method('generate') + ->will($this->returnValue($expectedReplaceCount > 0 ? ['test'] : [])); $this->urlPersist->expects($this->exactly($expectedReplaceCount)) ->method('replace') - ->with([1 => 'rewrite']); + ->with($expectedReplaceCount > 0 ? ['test'] : []); $this->deleteEntitiesFromStores->expects($this->any()) ->method('execute') diff --git a/app/code/Magento/UrlRewrite/Model/Storage/DeleteEntitiesFromStores.php b/app/code/Magento/UrlRewrite/Model/Storage/DeleteEntitiesFromStores.php index 8f56a382fc888..98ea968751182 100644 --- a/app/code/Magento/UrlRewrite/Model/Storage/DeleteEntitiesFromStores.php +++ b/app/code/Magento/UrlRewrite/Model/Storage/DeleteEntitiesFromStores.php @@ -18,12 +18,12 @@ class DeleteEntitiesFromStores { /** - * @var AdapterInterface $connection + * @var AdapterInterface */ private $connection; /** - * @var Resource $resource + * @var Resource */ private $resource; diff --git a/dev/tests/integration/testsuite/Magento/CatalogUrlRewrite/Observer/ProductProcessUrlRewriteSavingObserverTest.php b/dev/tests/integration/testsuite/Magento/CatalogUrlRewrite/Observer/ProductProcessUrlRewriteSavingObserverTest.php index eae90a47cead3..76508f2066b54 100644 --- a/dev/tests/integration/testsuite/Magento/CatalogUrlRewrite/Observer/ProductProcessUrlRewriteSavingObserverTest.php +++ b/dev/tests/integration/testsuite/Magento/CatalogUrlRewrite/Observer/ProductProcessUrlRewriteSavingObserverTest.php @@ -28,12 +28,12 @@ class ProductProcessUrlRewriteSavingObserverTest extends TestCase private $objectManager; /** - * @var StoreManagerInterface $storeManager + * @var StoreManagerInterface */ private $storeManager; /** - * @var ProductRepositoryInterface $productRepository + * @var ProductRepositoryInterface */ private $productRepository; @@ -372,7 +372,6 @@ public function testAddAndRemoveProductFromWebsite() /** * @magentoDataFixture Magento/Store/_files/second_website_with_two_stores.php * @magentoDataFixture Magento/CatalogUrlRewrite/_files/product_rewrite_multistore.php - * @magentoAppIsolation enabled * @SuppressWarnings(PHPMD.ExcessiveMethodLength) */ public function testChangeVisibilityGlobalScope() @@ -414,6 +413,7 @@ public function testChangeVisibilityGlobalScope() } //Set product to be not visible at global scope + $this->storeManager->setCurrentStore(Store::DEFAULT_STORE_ID); $product->setVisibility(Visibility::VISIBILITY_NOT_VISIBLE); $this->productRepository->save($product); $expected = []; @@ -425,6 +425,7 @@ public function testChangeVisibilityGlobalScope() //Add product to websites corresponding to all 4 stores. //Rewrites should not be present as the product is hidden //at the global scope. + $this->storeManager->setCurrentStore(Store::DEFAULT_STORE_ID); $product->setWebsiteIds( array_unique( [ @@ -443,6 +444,7 @@ public function testChangeVisibilityGlobalScope() } //Set product to be visible at global scope + $this->storeManager->setCurrentStore(Store::DEFAULT_STORE_ID); $product->setVisibility(Visibility::VISIBILITY_BOTH); $this->productRepository->save($product); $expected = [ @@ -484,7 +486,6 @@ public function testChangeVisibilityGlobalScope() /** * @magentoDataFixture Magento/Store/_files/second_website_with_two_stores.php * @magentoDataFixture Magento/CatalogUrlRewrite/_files/product_rewrite_multistore.php - * @magentoAppIsolation enabled * @SuppressWarnings(PHPMD.ExcessiveMethodLength) */ public function testChangeVisibilityLocalScope() From 7b2b5e3bd03b203cdac261a1100f2a3f9fa1aecf Mon Sep 17 00:00:00 2001 From: Graham Wharton Date: Fri, 6 Mar 2020 15:48:34 +0000 Subject: [PATCH 10/11] Fixed static test failures --- .../ProductProcessUrlRewriteSavingObserver.php | 1 + .../ProductProcessUrlRewriteSavingObserverTest.php | 12 ++++++------ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/app/code/Magento/CatalogUrlRewrite/Observer/ProductProcessUrlRewriteSavingObserver.php b/app/code/Magento/CatalogUrlRewrite/Observer/ProductProcessUrlRewriteSavingObserver.php index fddc0e49c6e2a..d97553f83f473 100644 --- a/app/code/Magento/CatalogUrlRewrite/Observer/ProductProcessUrlRewriteSavingObserver.php +++ b/app/code/Magento/CatalogUrlRewrite/Observer/ProductProcessUrlRewriteSavingObserver.php @@ -96,6 +96,7 @@ public function __construct( * @param Observer $observer * @return void * @throws UrlAlreadyExistsException + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ public function execute(Observer $observer) { diff --git a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/ProductProcessUrlRewriteSavingObserverTest.php b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/ProductProcessUrlRewriteSavingObserverTest.php index c2dcb1195bbce..2912fcb1b7fad 100644 --- a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/ProductProcessUrlRewriteSavingObserverTest.php +++ b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/ProductProcessUrlRewriteSavingObserverTest.php @@ -127,9 +127,10 @@ protected function setUp() $this->stores[2] = $this->initialiseStore(2, 2); $this->stores[5] = $this->initialiseStore(5, 2); - $this->product = $this->initialiseProduct($this->stores[0], 0); + $this->product = $this->initialiseProduct(); - $this->productCollection = $this->createPartialMock(Collection::class, + $this->productCollection = $this->createPartialMock( + Collection::class, ['getAllAttributeValues'] ); @@ -199,11 +200,9 @@ protected function setUp() /** * Initialise product for test * - * @param $store - * @param $storeId * @return MockObject */ - public function initialiseProduct($store, $storeId) + public function initialiseProduct() { $product = $this->createPartialMock( Product::class, @@ -253,6 +252,7 @@ public function initialiseStore($storeId, $websiteId) * Data provider * * @return array + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) */ public function urlKeyDataProvider() { @@ -477,9 +477,9 @@ public function urlKeyDataProvider() * @param int $productInWebsites * @param int $expectedReplaceCount * @param array $expectedRemoves + * @throws UrlAlreadyExistsException * * @dataProvider urlKeyDataProvider - * @throws UrlAlreadyExistsException */ public function testExecuteUrlKey( $productScope, From f363870fd5fc33917816531ee91df134978b3f7a Mon Sep 17 00:00:00 2001 From: Graham Wharton Date: Sat, 7 Mar 2020 17:38:41 +0000 Subject: [PATCH 11/11] Updated to use collectionFactory to generate collections instead of DI --- .../ProductProcessUrlRewriteSavingObserver.php | 15 ++++++++------- ...ProductProcessUrlRewriteSavingObserverTest.php | 15 ++++++++++++++- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/app/code/Magento/CatalogUrlRewrite/Observer/ProductProcessUrlRewriteSavingObserver.php b/app/code/Magento/CatalogUrlRewrite/Observer/ProductProcessUrlRewriteSavingObserver.php index d97553f83f473..377e19a22a31a 100644 --- a/app/code/Magento/CatalogUrlRewrite/Observer/ProductProcessUrlRewriteSavingObserver.php +++ b/app/code/Magento/CatalogUrlRewrite/Observer/ProductProcessUrlRewriteSavingObserver.php @@ -8,7 +8,7 @@ use Magento\Catalog\Api\Data\ProductInterface; use Magento\Catalog\Model\Product; use Magento\Catalog\Model\Product\Visibility; -use Magento\Catalog\Model\ResourceModel\Product\Collection; +use Magento\Catalog\Model\ResourceModel\Product\CollectionFactory; use Magento\CatalogUrlRewrite\Model\ProductScopeRewriteGenerator; use Magento\CatalogUrlRewrite\Model\ProductUrlPathGenerator; use Magento\CatalogUrlRewrite\Model\ProductUrlRewriteGenerator; @@ -59,9 +59,9 @@ class ProductProcessUrlRewriteSavingObserver implements ObserverInterface private $deleteEntitiesFromStores; /** - * @var Collection + * @var CollectionFactory */ - private $productCollection; + private $collectionFactory; /** * @param ProductUrlRewriteGenerator $productUrlRewriteGenerator @@ -70,7 +70,7 @@ class ProductProcessUrlRewriteSavingObserver implements ObserverInterface * @param StoreManagerInterface $storeManager * @param ProductScopeRewriteGenerator $productScopeRewriteGenerator * @param DeleteEntitiesFromStores $deleteEntitiesFromStores - * @param Collection $productCollection + * @param CollectionFactory $collectionFactory */ public function __construct( ProductUrlRewriteGenerator $productUrlRewriteGenerator, @@ -79,7 +79,7 @@ public function __construct( StoreManagerInterface $storeManager, ProductScopeRewriteGenerator $productScopeRewriteGenerator, DeleteEntitiesFromStores $deleteEntitiesFromStores, - Collection $productCollection + CollectionFactory $collectionFactory ) { $this->productUrlRewriteGenerator = $productUrlRewriteGenerator; $this->urlPersist = $urlPersist; @@ -87,7 +87,7 @@ public function __construct( $this->storeManager = $storeManager; $this->productScopeRewriteGenerator = $productScopeRewriteGenerator; $this->deleteEntitiesFromStores = $deleteEntitiesFromStores; - $this->productCollection = $productCollection; + $this->collectionFactory = $collectionFactory; } /** @@ -117,7 +117,8 @@ public function execute(Observer $observer) $storeIdsToRemove = []; $productWebsiteMap = array_flip($product->getWebsiteIds()); - $storeVisibilities = $this->productCollection->getAllAttributeValues(ProductInterface::VISIBILITY); + $storeVisibilities = $this->collectionFactory->create() + ->getAllAttributeValues(ProductInterface::VISIBILITY); if ($this->productScopeRewriteGenerator->isGlobalScope($product->getStoreId())) { //Remove any rewrite URLs for websites the product is not in, or is not visible in. Global Scope. foreach ($this->storeManager->getStores() as $store) { diff --git a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/ProductProcessUrlRewriteSavingObserverTest.php b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/ProductProcessUrlRewriteSavingObserverTest.php index 2912fcb1b7fad..602bfabcd45f2 100644 --- a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/ProductProcessUrlRewriteSavingObserverTest.php +++ b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/ProductProcessUrlRewriteSavingObserverTest.php @@ -7,6 +7,7 @@ namespace Magento\CatalogUrlRewrite\Test\Unit\Observer; use Magento\Catalog\Model\ResourceModel\Product\Collection; +use Magento\Catalog\Model\ResourceModel\Product\CollectionFactory; use Magento\CatalogUrlRewrite\Model\ProductScopeRewriteGenerator; use Magento\CatalogUrlRewrite\Model\ProductUrlRewriteGenerator; use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; @@ -106,6 +107,11 @@ class ProductProcessUrlRewriteSavingObserverTest extends TestCase */ private $productCollection; + /** + * @var CollectionFactory|MockObject + */ + private $collectionFactory; + /** * Set up * Website_ID = 0 -> Store_ID = 0 @@ -129,10 +135,17 @@ protected function setUp() $this->product = $this->initialiseProduct(); + $this->collectionFactory = $this->createPartialMock( + CollectionFactory::class, + ['create'] + ); $this->productCollection = $this->createPartialMock( Collection::class, ['getAllAttributeValues'] ); + $this->collectionFactory->expects($this->any()) + ->method('create') + ->willReturn($this->productCollection); $this->deleteEntitiesFromStores = $this->createPartialMock( DeleteEntitiesFromStores::class, @@ -192,7 +205,7 @@ protected function setUp() 'storeWebsiteRelation' => $this->storeWebsiteRelation, 'deleteEntitiesFromStores' => $this->deleteEntitiesFromStores, 'productScopeRewriteGenerator' => $this->productScopeRewriteGenerator, - 'productCollection' => $this->productCollection + 'collectionFactory' => $this->collectionFactory ] ); }