From 17236cc014ab718161e0cf446ce40f4eb8d27d9e Mon Sep 17 00:00:00 2001 From: Lukasz Bajsarowicz Date: Sat, 29 Feb 2020 11:41:57 +0100 Subject: [PATCH 1/4] #27089 Fix issue with returning non-available default limit, coverage with Unit Tests --- .../Catalog/Helper/Product/ProductList.php | 107 +++++++----------- .../Unit/Helper/Product/ProductListTest.php | 74 ++++++++++++ 2 files changed, 118 insertions(+), 63 deletions(-) create mode 100644 app/code/Magento/Catalog/Test/Unit/Helper/Product/ProductListTest.php diff --git a/app/code/Magento/Catalog/Helper/Product/ProductList.php b/app/code/Magento/Catalog/Helper/Product/ProductList.php index 3aa6aeed3779a..3b0a3d12702f5 100644 --- 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,34 +108,28 @@ 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; @@ -149,24 +137,17 @@ public function getAvailableLimit($mode) } /** - * 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 new file mode 100644 index 0000000000000..176fb90932e01 --- /dev/null +++ b/app/code/Magento/Catalog/Test/Unit/Helper/Product/ProductListTest.php @@ -0,0 +1,74 @@ +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 + ] + ]; + } +} From 381ab640c0039652e93c2be24f0a9bf6be9a883e Mon Sep 17 00:00:00 2001 From: Lukasz Bajsarowicz Date: Sat, 29 Feb 2020 13:24:04 +0100 Subject: [PATCH 2/4] Fix Static Tests --- app/code/Magento/Catalog/Helper/Product/ProductList.php | 2 ++ .../Catalog/Test/Unit/Helper/Product/ProductListTest.php | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/code/Magento/Catalog/Helper/Product/ProductList.php b/app/code/Magento/Catalog/Helper/Product/ProductList.php index 3b0a3d12702f5..047b2a9567058 100644 --- a/app/code/Magento/Catalog/Helper/Product/ProductList.php +++ b/app/code/Magento/Catalog/Helper/Product/ProductList.php @@ -14,6 +14,8 @@ use Magento\Store\Model\ScopeInterface; /** + * Returns data for toolbars of Sorting and Pagination + * * @api * @since 100.0.2 */ diff --git a/app/code/Magento/Catalog/Test/Unit/Helper/Product/ProductListTest.php b/app/code/Magento/Catalog/Test/Unit/Helper/Product/ProductListTest.php index 176fb90932e01..9d4e3527fd34d 100644 --- a/app/code/Magento/Catalog/Test/Unit/Helper/Product/ProductListTest.php +++ b/app/code/Magento/Catalog/Test/Unit/Helper/Product/ProductListTest.php @@ -7,7 +7,6 @@ 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; From 778fbfbe8bd2740713762220f61eac07e34deedb Mon Sep 17 00:00:00 2001 From: Lukasz Bajsarowicz Date: Sat, 29 Feb 2020 13:27:32 +0100 Subject: [PATCH 3/4] Deprecate `const` --- .../Magento/Catalog/Helper/Product/ProductList.php | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/app/code/Magento/Catalog/Helper/Product/ProductList.php b/app/code/Magento/Catalog/Helper/Product/ProductList.php index 047b2a9567058..b7fca6f58ab20 100644 --- a/app/code/Magento/Catalog/Helper/Product/ProductList.php +++ b/app/code/Magento/Catalog/Helper/Product/ProductList.php @@ -21,11 +21,19 @@ */ class ProductList { + public const XML_PATH_LIST_MODE = 'catalog/frontend/list_mode'; + public const DEFAULT_SORT_DIRECTION = 'asc'; + /** - * List mode configuration path + * @deprecated */ - const XML_PATH_LIST_MODE = 'catalog/frontend/list_mode'; - const DEFAULT_SORT_DIRECTION = 'asc'; + const VIEW_MODE_LIST = 'list'; + + /** + * @deprecated + */ + const VIEW_MODE_GRID = 'grid'; + /** * @var ScopeConfigInterface From d3962f2e82532829522e5cf68122b92c27932a02 Mon Sep 17 00:00:00 2001 From: Lukasz Bajsarowicz Date: Sat, 29 Feb 2020 14:05:43 +0100 Subject: [PATCH 4/4] ... :facepalm: --- app/code/Magento/Catalog/Helper/Product/ProductList.php | 8 -------- 1 file changed, 8 deletions(-) diff --git a/app/code/Magento/Catalog/Helper/Product/ProductList.php b/app/code/Magento/Catalog/Helper/Product/ProductList.php index b7fca6f58ab20..9f976654d9676 100644 --- a/app/code/Magento/Catalog/Helper/Product/ProductList.php +++ b/app/code/Magento/Catalog/Helper/Product/ProductList.php @@ -24,17 +24,9 @@ class ProductList public const XML_PATH_LIST_MODE = 'catalog/frontend/list_mode'; public const DEFAULT_SORT_DIRECTION = 'asc'; - /** - * @deprecated - */ const VIEW_MODE_LIST = 'list'; - - /** - * @deprecated - */ const VIEW_MODE_GRID = 'grid'; - /** * @var ScopeConfigInterface */