Processed review comments

Author: Graham Wharton

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]);
     }
 }

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
+                );
             }
         }
     }