#27089 Fix issue with returning non-available default limit, coverage…

… with Unit Tests
magento · Feb 29, 2020 · 17236cc · 17236cc
1 parent a3c2af0
commit 17236cc
Show file tree

Hide file tree

Showing 2 changed files with 118 additions and 63 deletions.
diff --git a/app/code/Magento/Catalog/Helper/Product/ProductList.php b/app/code/Magento/Catalog/Helper/Product/ProductList.php
@@ -3,12 +3,17 @@
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
+declare(strict_types=1);
 
 namespace Magento\Catalog\Helper\Product;
 
+use Magento\Catalog\Model\Config;
+use Magento\Framework\App\Config\ScopeConfigInterface;
+use Magento\Framework\App\ObjectManager;
+use Magento\Framework\Registry;
+use Magento\Store\Model\ScopeInterface;
+
 /**
- * Class ProductList
- *
  * @api
  * @since 100.0.2
  */
@@ -18,18 +23,15 @@ class ProductList
      * List mode configuration path
      */
     const XML_PATH_LIST_MODE = 'catalog/frontend/list_mode';
-
-    const VIEW_MODE_LIST = 'list';
-    const VIEW_MODE_GRID = 'grid';
-
     const DEFAULT_SORT_DIRECTION = 'asc';
+
     /**
-     * @var \Magento\Framework\App\Config\ScopeConfigInterface
+     * @var ScopeConfigInterface
      */
     protected $scopeConfig;
 
     /**
-     * @var \Magento\Framework\Registry
+     * @var Registry
      */
     private $coreRegistry;
 
@@ -38,20 +40,18 @@ class ProductList
      *
      * @var array
      */
-    protected $_defaultAvailableLimit  = [10 => 10,20 => 20,50 => 50];
+    protected $_defaultAvailableLimit = [10 => 10, 20 => 20, 50 => 50];
 
     /**
-     * @param \Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig
-     * @param \Magento\Framework\Registry $coreRegistry
+     * @param ScopeConfigInterface $scopeConfig
+     * @param Registry $coreRegistry
      */
     public function __construct(
-        \Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig,
-        \Magento\Framework\Registry $coreRegistry = null
+        ScopeConfigInterface $scopeConfig,
+        Registry $coreRegistry = null
     ) {
         $this->scopeConfig = $scopeConfig;
-        $this->coreRegistry = $coreRegistry ?: \Magento\Framework\App\ObjectManager::getInstance()->get(
-            \Magento\Framework\Registry::class
-        );
+        $this->coreRegistry = $coreRegistry ?? ObjectManager::getInstance()->get(Registry::class);
     }
 
     /**
@@ -61,31 +61,23 @@ public function __construct(
      */
     public function getAvailableViewMode()
     {
-        $value = $this->scopeConfig->getValue(
-            self::XML_PATH_LIST_MODE,
-            \Magento\Store\Model\ScopeInterface::SCOPE_STORE
-        );
+        $value = $this->scopeConfig->getValue(self::XML_PATH_LIST_MODE, ScopeInterface::SCOPE_STORE);
+
         switch ($value) {
             case 'grid':
-                $availableMode = ['grid' => __('Grid')];
-                break;
+                return ['grid' => __('Grid')];
 
             case 'list':
-                $availableMode = ['list' => __('List')];
-                break;
+                return ['list' => __('List')];
 
             case 'grid-list':
-                $availableMode = ['grid' => __('Grid'), 'list' =>  __('List')];
-                break;
+                return ['grid' => __('Grid'), 'list' => __('List')];
 
             case 'list-grid':
-                $availableMode = ['list' => __('List'), 'grid' => __('Grid')];
-                break;
-            default:
-                $availableMode = null;
-                break;
+                return ['list' => __('List'), 'grid' => __('Grid')];
         }
-        return $availableMode;
+
+        return null;
     }
 
     /**
@@ -99,12 +91,14 @@ public function getDefaultViewMode($options = [])
         if (empty($options)) {
             $options = $this->getAvailableViewMode();
         }
+
         return current(array_keys($options));
     }
 
     /**
      * Get default sort field
      *
+     * @FIXME Helper should be context-independent
      * @return null|string
      */
     public function getDefaultSortField()
@@ -114,59 +108,46 @@ public function getDefaultSortField()
             return $currentCategory->getDefaultSortBy();
         }
 
-        return $this->scopeConfig->getValue(
-            \Magento\Catalog\Model\Config::XML_PATH_LIST_DEFAULT_SORT_BY,
-            \Magento\Store\Model\ScopeInterface::SCOPE_STORE
-        );
+        return $this->scopeConfig->getValue(Config::XML_PATH_LIST_DEFAULT_SORT_BY, ScopeInterface::SCOPE_STORE);
     }
 
     /**
      * Retrieve available limits for specified view mode
      *
-     * @param string $mode
+     * @param string $viewMode
      * @return array
      */
-    public function getAvailableLimit($mode)
+    public function getAvailableLimit($viewMode): array
     {
-        if (!in_array($mode, [self::VIEW_MODE_GRID, self::VIEW_MODE_LIST])) {
+        $availableViewModes = $this->getAvailableViewMode();
+
+        if (!isset($availableViewModes[$viewMode])) {
             return $this->_defaultAvailableLimit;
         }
-        $perPageConfigKey = 'catalog/frontend/' . $mode . '_per_page_values';
-        $perPageValues = (string)$this->scopeConfig->getValue(
-            $perPageConfigKey,
-            \Magento\Store\Model\ScopeInterface::SCOPE_STORE
-        );
+
+        $perPageConfigPath = 'catalog/frontend/' . $viewMode . '_per_page_values';
+        $perPageValues = (string)$this->scopeConfig->getValue($perPageConfigPath, ScopeInterface::SCOPE_STORE);
         $perPageValues = explode(',', $perPageValues);
         $perPageValues = array_combine($perPageValues, $perPageValues);
-        if ($this->scopeConfig->isSetFlag(
-            'catalog/frontend/list_allow_all',
-            \Magento\Store\Model\ScopeInterface::SCOPE_STORE
-        )) {
+        if ($this->scopeConfig->isSetFlag('catalog/frontend/list_allow_all', ScopeInterface::SCOPE_STORE)) {
             return ($perPageValues + ['all' => __('All')]);
         } else {
             return $perPageValues;
         }
     }
 
     /**
-     * Retrieve default per page values
+     * Returns default value of `per_page` for view mode provided
      *
      * @param string $viewMode
-     * @return string (comma separated)
+     * @return int
      */
-    public function getDefaultLimitPerPageValue($viewMode)
+    public function getDefaultLimitPerPageValue($viewMode): int
     {
-        if ($viewMode == self::VIEW_MODE_LIST) {
-            return $this->scopeConfig->getValue(
-                'catalog/frontend/list_per_page',
-                \Magento\Store\Model\ScopeInterface::SCOPE_STORE
-            );
-        } elseif ($viewMode == self::VIEW_MODE_GRID) {
-            return $this->scopeConfig->getValue(
-                'catalog/frontend/grid_per_page',
-                \Magento\Store\Model\ScopeInterface::SCOPE_STORE
-            );
-        }
-        return 0;
+        $xmlConfigPath = sprintf('catalog/frontend/%s_per_page', $viewMode);
+        $defaultLimit = $this->scopeConfig->getValue($xmlConfigPath, ScopeInterface::SCOPE_STORE);
+
+        $availableLimits = $this->getAvailableLimit($viewMode);
+        return (int)($availableLimits[$defaultLimit] ?? current($availableLimits));
     }
 }
diff --git a/app/code/Magento/Catalog/Test/Unit/Helper/Product/ProductListTest.php b/app/code/Magento/Catalog/Test/Unit/Helper/Product/ProductListTest.php
@@ -0,0 +1,74 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+declare(strict_types=1);
+
+namespace Magento\Catalog\Test\Unit\Helper\Product;
+
+
+use Magento\Catalog\Helper\Product\ProductList;
+use Magento\Framework\App\Config\ScopeConfigInterface;
+use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
+use PHPUnit\Framework\MockObject\MockObject;
+use PHPUnit\Framework\TestCase;
+
+class ProductListTest extends TestCase
+{
+    const STUB_VIEW_MODE = 'grid';
+    /**
+     * @var ScopeConfigInterface|MockObject
+     */
+    private $scopeConfigMock;
+
+    /**
+     * @var ProductList
+     */
+    private $productListHelper;
+
+    protected function setUp()
+    {
+        $objectManager = new ObjectManager($this);
+
+        $this->scopeConfigMock = $this->getMockForAbstractClass(ScopeConfigInterface::class);
+        $this->productListHelper = $objectManager->getObject(ProductList::class, [
+            'scopeConfig' => $this->scopeConfigMock
+        ]);
+    }
+
+    /**
+     * @dataProvider defaultAvailableLimitsDataProvider
+     */
+    public function testGetDefaultLimitPerPageValueReturnsOneOfAvailableLimits(
+        string $availableValues,
+        int $defaultValue,
+        int $expectedReturn
+    ) {
+        $this->scopeConfigMock->method('getValue')
+            ->willReturnMap([
+                [sprintf('catalog/frontend/%s_per_page_values', self::STUB_VIEW_MODE), $availableValues],
+                [sprintf('catalog/frontend/%s_per_page', self::STUB_VIEW_MODE), $defaultValue]
+            ]);
+
+        $returnedValue = $this->productListHelper->getDefaultLimitPerPageValue(self::STUB_VIEW_MODE);
+
+        $this->assertSame($expectedReturn, $returnedValue);
+    }
+
+    public function defaultAvailableLimitsDataProvider(): array
+    {
+        return [
+            'limit-available' => [
+                'values' => '10,20,30',
+                'default' => 10,
+                'expected' => 10
+            ],
+            'limit-not-available' => [
+                'values' => '10,20,30',
+                'default' => 1,
+                'expected' => 10
+            ]
+        ];
+    }
+}