Updated code as per reviewer

magento · Jul 7, 2020 · d59d740 · d59d740
1 parent e100822
commit d59d740
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 14 deletions.
diff --git a/app/code/Magento/CatalogUrlRewrite/Model/ProductScopeRewriteGenerator.php b/app/code/Magento/CatalogUrlRewrite/Model/ProductScopeRewriteGenerator.php
@@ -6,6 +6,7 @@
 namespace Magento\CatalogUrlRewrite\Model;
 
 use Magento\Catalog\Api\CategoryRepositoryInterface;
+use Magento\Catalog\Api\Data\CategoryInterface;
 use Magento\Catalog\Model\Category;
 use Magento\Catalog\Model\Product;
 use Magento\CatalogUrlRewrite\Model\Product\AnchorUrlRewriteGenerator;
@@ -15,13 +16,12 @@
 use Magento\CatalogUrlRewrite\Service\V1\StoreViewService;
 use Magento\Framework\App\Config\ScopeConfigInterface;
 use Magento\Framework\App\ObjectManager;
+use Magento\Framework\Exception\NoSuchEntityException;
 use Magento\Store\Model\Store;
 use Magento\Store\Model\StoreManagerInterface;
 use Magento\UrlRewrite\Model\MergeDataProviderFactory;
 
 /**
- * Class ProductScopeRewriteGenerator
- *
  * Generates Product/Category URLs for different scopes
  *
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
@@ -237,14 +237,18 @@ public function isCategoryProperForGenerating(Category $category, $storeId)
     }
 
     /**
-     * Check if URL key has been changed
-     *
      * Checks if URL key has been changed for provided category and returns reloaded category,
      * in other case - returns provided category.
      *
+     * Category should be loaded per appropriate store at all times. This is because whilst the URL key on the
+     * category in focus might be unchanged, parent category URL keys might be. If the category store ID
+     * and passed store ID are the same then return current category as it is correct but may have changed in memory
+     *
      * @param int $storeId
      * @param Category $category
-     * @return Category
+     *
+     * @return CategoryInterface
+     * @throws NoSuchEntityException
      */
     private function getCategoryWithOverriddenUrlKey($storeId, Category $category)
     {
@@ -254,11 +258,7 @@ private function getCategoryWithOverriddenUrlKey($storeId, Category $category)
             Category::ENTITY
         );
 
-        // Category should be loaded per appropriate store at all times. This is because whilst the URL key on the
-        // category in focus might be unchanged, parent category URL keys might be. If the category store ID
-        // and passed store ID are the same then return current category as it is correct but may have changed in memory
-
-        if (!$isUrlKeyOverridden && $storeId == $category->getStoreId()) {
+        if (!$isUrlKeyOverridden && $storeId === $category->getStoreId()) {
             return $category;
         }
 

diff --git a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/ProductScopeRewriteGeneratorTest.php b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/ProductScopeRewriteGeneratorTest.php
@@ -71,7 +71,7 @@ class ProductScopeRewriteGeneratorTest extends TestCase
     private $configMock;
 
     /** @var CategoryRepositoryInterface|MockObject */
-    private $categoryRepository;
+    private $categoryRepositoryMock;
 
     protected function setUp(): void
     {
@@ -130,7 +130,7 @@ function ($value) {
         $this->configMock = $this->getMockBuilder(ScopeConfigInterface::class)
             ->getMock();
 
-        $this->categoryRepository = $this->getMockForAbstractClass(CategoryRepositoryInterface::class);
+        $this->categoryRepositoryMock = $this->getMockForAbstractClass(CategoryRepositoryInterface::class);
 
         $this->productScopeGenerator = (new ObjectManager($this))->getObject(
             ProductScopeRewriteGenerator::class,
@@ -144,7 +144,7 @@ function ($value) {
                 'storeManager' => $this->storeManager,
                 'mergeDataProviderFactory' => $mergeDataProviderFactory,
                 'config' => $this->configMock,
-                'categoryRepository' => $this->categoryRepository
+                'categoryRepository' => $this->categoryRepositoryMock
             ]
         );
         $this->categoryMock = $this->getMockBuilder(Category::class)
@@ -222,7 +222,7 @@ public function testGenerationForSpecificStore()
         $this->anchorUrlRewriteGenerator->expects($this->any())->method('generate')
             ->willReturn([]);
 
-        $this->categoryRepository->expects($this->once())->method('get')->willReturn($this->categoryMock);
+        $this->categoryRepositoryMock->expects($this->once())->method('get')->willReturn($this->categoryMock);
 
         $this->assertEquals(
             ['category-1_1' => $canonical],