From 1b9f190d18ec1346a9527b26cb7ead7f2019a439 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Szubert?= Date: Fri, 29 Jan 2021 00:20:46 +0100 Subject: [PATCH] Fix #30471 - Page layouts are hard-coded in Magento\Widget\Block\Adminhtml\Widget\Instance\Edit\Chooser\Container --- .../Instance/Edit/Chooser/Container.php | 56 +++++++++++++------ .../Edit/Chooser/AbstractContainerTest.php | 25 +++++++-- .../Instance/Edit/Chooser/ContainerTest.php | 13 ++--- 3 files changed, 64 insertions(+), 30 deletions(-) diff --git a/app/code/Magento/Widget/Block/Adminhtml/Widget/Instance/Edit/Chooser/Container.php b/app/code/Magento/Widget/Block/Adminhtml/Widget/Instance/Edit/Chooser/Container.php index 068da79dc196c..cdc56b6c9ccf9 100644 --- a/app/code/Magento/Widget/Block/Adminhtml/Widget/Instance/Edit/Chooser/Container.php +++ b/app/code/Magento/Widget/Block/Adminhtml/Widget/Instance/Edit/Chooser/Container.php @@ -3,8 +3,17 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ +declare(strict_types=1); + namespace Magento\Widget\Block\Adminhtml\Widget\Instance\Edit\Chooser; +use Magento\Framework\App\ObjectManager; +use Magento\Framework\View\Element\Context; +use Magento\Framework\View\Element\Html\Select; +use Magento\Framework\View\Layout\ProcessorFactory; +use Magento\Framework\View\Model\PageLayout\Config\BuilderInterface as PageLayoutConfigBuilder; +use Magento\Theme\Model\ResourceModel\Theme\CollectionFactory; + /** * A chooser for container for widget instances * @@ -13,10 +22,12 @@ * @method \Magento\Widget\Block\Adminhtml\Widget\Instance\Edit\Chooser\Container setTheme($theme) * @method \Magento\Widget\Block\Adminhtml\Widget\Instance\Edit\Chooser\Container setArea($area) */ -class Container extends \Magento\Framework\View\Element\Html\Select +class Container extends Select { /**#@+ * Frontend page layouts + * @deprecated hardcoded list was replaced with checking actual existing layouts + * @see \Magento\Framework\View\Model\PageLayout\Config\BuilderInterface::getPageLayoutsConfig */ const PAGE_LAYOUT_1COLUMN = '1column-center'; const PAGE_LAYOUT_2COLUMNS_LEFT = '2columns-left'; @@ -24,29 +35,40 @@ class Container extends \Magento\Framework\View\Element\Html\Select const PAGE_LAYOUT_3COLUMNS = '3columns'; /**#@-*/ - /**#@-*/ + /** + * @var ProcessorFactory + */ protected $_layoutProcessorFactory; /** - * @var \Magento\Theme\Model\ResourceModel\Theme\CollectionFactory + * @var CollectionFactory */ protected $_themesFactory; /** - * @param \Magento\Framework\View\Element\Context $context - * @param \Magento\Framework\View\Layout\ProcessorFactory $layoutProcessorFactory - * @param \Magento\Theme\Model\ResourceModel\Theme\CollectionFactory $themesFactory + * @var PageLayoutConfigBuilder + */ + private $pageLayoutConfigBuilder; + + /** + * @param Context $context + * @param ProcessorFactory $layoutProcessorFactory + * @param CollectionFactory $themesFactory * @param array $data + * @param PageLayoutConfigBuilder|null $pageLayoutConfigBuilder */ public function __construct( - \Magento\Framework\View\Element\Context $context, - \Magento\Framework\View\Layout\ProcessorFactory $layoutProcessorFactory, - \Magento\Theme\Model\ResourceModel\Theme\CollectionFactory $themesFactory, - array $data = [] + Context $context, + ProcessorFactory $layoutProcessorFactory, + CollectionFactory $themesFactory, + array $data = [], + PageLayoutConfigBuilder $pageLayoutConfigBuilder = null ) { + parent::__construct($context, $data); $this->_layoutProcessorFactory = $layoutProcessorFactory; $this->_themesFactory = $themesFactory; - parent::__construct($context, $data); + $this->pageLayoutConfigBuilder = $pageLayoutConfigBuilder + ?? ObjectManager::getInstance()->get(PageLayoutConfigBuilder::class); } /** @@ -101,6 +123,7 @@ protected function _beforeToHtml() $this->addOption($containerName, $containerLabel); } } + return parent::_beforeToHtml(); } @@ -108,12 +131,14 @@ protected function _beforeToHtml() * Retrieve theme instance by its identifier * * @param int $themeId + * * @return \Magento\Theme\Model\Theme|null */ protected function _getThemeInstance($themeId) { /** @var \Magento\Theme\Model\ResourceModel\Theme\Collection $themeCollection */ $themeCollection = $this->_themesFactory->create(); + return $themeCollection->getItemById($themeId); } @@ -124,11 +149,8 @@ protected function _getThemeInstance($themeId) */ protected function getPageLayouts() { - return [ - self::PAGE_LAYOUT_1COLUMN, - self::PAGE_LAYOUT_2COLUMNS_LEFT, - self::PAGE_LAYOUT_2COLUMNS_RIGHT, - self::PAGE_LAYOUT_3COLUMNS, - ]; + $pageLayoutsConfig = $this->pageLayoutConfigBuilder->getPageLayoutsConfig(); + + return array_keys($pageLayoutsConfig->getPageLayouts()); } } diff --git a/app/code/Magento/Widget/Test/Unit/Block/Adminhtml/Widget/Instance/Edit/Chooser/AbstractContainerTest.php b/app/code/Magento/Widget/Test/Unit/Block/Adminhtml/Widget/Instance/Edit/Chooser/AbstractContainerTest.php index 821400183ed33..3ef1d96d45b70 100644 --- a/app/code/Magento/Widget/Test/Unit/Block/Adminhtml/Widget/Instance/Edit/Chooser/AbstractContainerTest.php +++ b/app/code/Magento/Widget/Test/Unit/Block/Adminhtml/Widget/Instance/Edit/Chooser/AbstractContainerTest.php @@ -12,15 +12,19 @@ use Magento\Framework\App\Config\ScopeConfigInterface; use Magento\Framework\Escaper; use Magento\Framework\Event\Manager; -use Magento\Framework\TestFramework\Unit\Helper\ObjectManager as ObjectManagerHelper; use Magento\Framework\View\Layout\ProcessorFactory; use Magento\Framework\View\Model\Layout\Merge; +use Magento\Framework\View\Model\PageLayout\Config\BuilderInterface as PageLayoutConfigBuilder; +use Magento\Framework\View\PageLayout\Config as PageLayoutConfig; use Magento\Theme\Model\ResourceModel\Theme\Collection; use Magento\Theme\Model\ResourceModel\Theme\CollectionFactory; use Magento\Theme\Model\Theme; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +/** + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) + */ abstract class AbstractContainerTest extends TestCase { /** @@ -69,17 +73,15 @@ abstract class AbstractContainerTest extends TestCase protected $escaperMock; /** - * @var ObjectManagerHelper + * @var PageLayoutConfigBuilder|MockObject */ - protected $objectManagerHelper; + protected $pageLayoutConfigBuilderMock; /** * @return void */ protected function setUp(): void { - $this->objectManagerHelper = new ObjectManagerHelper($this); - $this->eventManagerMock = $this->getMockBuilder(Manager::class) ->setMethods(['dispatch']) ->disableOriginalConstructor() @@ -116,7 +118,7 @@ protected function setUp(): void Escaper::class, ['escapeHtml', 'escapeHtmlAttr'] ); - $this->escaperMock->expects($this->any())->method('escapeHtmlAttr')->willReturnArgument(0); + $this->escaperMock->method('escapeHtmlAttr')->willReturnArgument(0); $this->contextMock = $this->getMockBuilder(Context::class) ->setMethods(['getEventManager', 'getScopeConfig', 'getEscaper']) @@ -125,5 +127,16 @@ protected function setUp(): void $this->contextMock->expects($this->once())->method('getEventManager')->willReturn($this->eventManagerMock); $this->contextMock->expects($this->once())->method('getScopeConfig')->willReturn($this->scopeConfigMock); $this->contextMock->expects($this->once())->method('getEscaper')->willReturn($this->escaperMock); + + $this->pageLayoutConfigBuilderMock = $this->getMockBuilder(PageLayoutConfigBuilder::class) + ->getMockForAbstractClass(); + $pageLayoutConfigMock = $this->getMockBuilder(PageLayoutConfig::class) + ->onlyMethods(['getPageLayouts']) + ->disableOriginalConstructor() + ->getMock(); + $pageLayoutConfigMock->method('getPageLayouts') + ->willReturn(['empty' => 'Empty']); + $this->pageLayoutConfigBuilderMock->method('getPageLayoutsConfig') + ->willReturn($pageLayoutConfigMock); } } diff --git a/app/code/Magento/Widget/Test/Unit/Block/Adminhtml/Widget/Instance/Edit/Chooser/ContainerTest.php b/app/code/Magento/Widget/Test/Unit/Block/Adminhtml/Widget/Instance/Edit/Chooser/ContainerTest.php index 531ca121d7cfd..8eba9595ef20a 100644 --- a/app/code/Magento/Widget/Test/Unit/Block/Adminhtml/Widget/Instance/Edit/Chooser/ContainerTest.php +++ b/app/code/Magento/Widget/Test/Unit/Block/Adminhtml/Widget/Instance/Edit/Chooser/ContainerTest.php @@ -23,13 +23,12 @@ protected function setUp(): void { parent::setUp(); - $this->containerBlock = $this->objectManagerHelper->getObject( - Container::class, - [ - 'context' => $this->contextMock, - 'themesFactory' => $this->themeCollectionFactoryMock, - 'layoutProcessorFactory' => $this->layoutProcessorFactoryMock - ] + $this->containerBlock = new Container( + $this->contextMock, + $this->layoutProcessorFactoryMock, + $this->themeCollectionFactoryMock, + [], + $this->pageLayoutConfigBuilderMock ); }