ENGCOM-7133: Implement ActionInterface for \cms/page/view\ #27298

magento-engcom-team · magento-engcom-team · commit 614fb6978f06 · 2020-03-17T15:39:28.000-05:00
- Merge Pull Request #27298 from lbajsarowicz/magento2:refactor/cms-page-view - Merged commits: 1. f49060b 2. 4fd4f99 3. 4a7adeb
diff --git a/app/code/Magento/Cms/Controller/Page/View.php b/app/code/Magento/Cms/Controller/Page/View.php
@@ -1,50 +1,78 @@
 <?php
 /**
- *
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
+declare(strict_types=1);
+
 namespace Magento\Cms\Controller\Page;
 
-use Magento\Framework\App\Action\HttpPostActionInterface;
+use Magento\Cms\Helper\Page as PageHelper;
 use Magento\Framework\App\Action\HttpGetActionInterface;
-use Magento\Framework\App\Action\Action;
+use Magento\Framework\App\Action\HttpPostActionInterface;
+use Magento\Framework\App\RequestInterface;
+use Magento\Framework\Controller\Result\ForwardFactory;
+use Magento\Framework\Controller\ResultInterface;
 
 /**
  * Custom page for storefront. Needs to be accessible by POST because of the store switching.
  */
-class View extends Action implements HttpGetActionInterface, HttpPostActionInterface
+class View implements HttpGetActionInterface, HttpPostActionInterface
 {
     /**
-     * @var \Magento\Framework\Controller\Result\ForwardFactory
+     * @var ForwardFactory
      */
     protected $resultForwardFactory;
 
     /**
-     * @param \Magento\Framework\App\Action\Context $context
-     * @param \Magento\Framework\Controller\Result\ForwardFactory $resultForwardFactory
+     * @var RequestInterface
+     */
+    private $request;
+
+    /**
+     * @var PageHelper
+     */
+    private $pageHelper;
+
+    /**
+     * @param RequestInterface $request
+     * @param PageHelper $pageHelper
+     * @param ForwardFactory $resultForwardFactory
      */
     public function __construct(
-        \Magento\Framework\App\Action\Context $context,
-        \Magento\Framework\Controller\Result\ForwardFactory $resultForwardFactory
+        RequestInterface $request,
+        PageHelper $pageHelper,
+        ForwardFactory $resultForwardFactory
     ) {
+        $this->request = $request;
+        $this->pageHelper = $pageHelper;
         $this->resultForwardFactory = $resultForwardFactory;
-        parent::__construct($context);
     }
 
     /**
      * View CMS page action
      *
-     * @return \Magento\Framework\Controller\ResultInterface
+     * @return ResultInterface
      */
     public function execute()
     {
-        $pageId = $this->getRequest()->getParam('page_id', $this->getRequest()->getParam('id', false));
-        $resultPage = $this->_objectManager->get(\Magento\Cms\Helper\Page::class)->prepareResultPage($this, $pageId);
+        $resultPage = $this->pageHelper->prepareResultPage($this, $this->getPageId());
         if (!$resultPage) {
             $resultForward = $this->resultForwardFactory->create();
             return $resultForward->forward('noroute');
         }
         return $resultPage;
     }
+
+    /**
+     * Returns Page ID if provided or null
+     *
+     * @return int|null
+     */
+    private function getPageId(): ?int
+    {
+        $id = $this->request->getParam('page_id') ?? $this->request->getParam('id');
+
+        return $id ? (int)$id : null;
+    }
 }
diff --git a/app/code/Magento/Cms/Helper/Page.php b/app/code/Magento/Cms/Helper/Page.php
@@ -8,17 +8,20 @@
 use Magento\Cms\Model\Page\CustomLayoutManagerInterface;
 use Magento\Cms\Model\Page\CustomLayoutRepositoryInterface;
 use Magento\Cms\Model\Page\IdentityMap;
-use Magento\Framework\App\Action\Action;
+use Magento\Framework\App\ActionInterface;
+use Magento\Framework\App\Helper\AbstractHelper;
 use Magento\Framework\App\ObjectManager;
 use Magento\Framework\Exception\NoSuchEntityException;
+use Magento\Framework\View\Result\Page as ResultPage;
 
 /**
  * CMS Page Helper
+ *
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
  * @SuppressWarnings(PHPMD.CyclomaticComplexity)
  * @SuppressWarnings(PHPMD.NPathComplexity)
  */
-class Page extends \Magento\Framework\App\Helper\AbstractHelper
+class Page extends AbstractHelper
 {
     /**
      * CMS no-route config path
@@ -146,14 +149,14 @@ public function __construct(
     /**
      * Return result CMS page
      *
-     * @param Action $action
+     * @param ActionInterface $action
      * @param int $pageId
-     * @return \Magento\Framework\View\Result\Page|bool
+     * @return ResultPage|bool
      */
-    public function prepareResultPage(Action $action, $pageId = null)
+    public function prepareResultPage(ActionInterface $action, $pageId = null)
     {
         if ($pageId !== null && $pageId !== $this->_page->getId()) {
-            $delimiterPosition = strrpos($pageId, '|');
+            $delimiterPosition = strrpos((string)$pageId, '|');
             if ($delimiterPosition) {
                 $pageId = substr($pageId, 0, $delimiterPosition);
             }
@@ -180,7 +183,7 @@ public function prepareResultPage(Action $action, $pageId = null)
                 $this->_design->setDesignTheme($this->_page->getCustomTheme());
             }
         }
-        /** @var \Magento\Framework\View\Result\Page $resultPage */
+        /** @var ResultPage $resultPage */
         $resultPage = $this->resultPageFactory->create();
         $this->setLayoutType($inRange, $resultPage);
         $resultPage->addHandle('cms_page_view');
@@ -247,8 +250,8 @@ public function getPageUrl($pageId = null)
      * Set layout type
      *
      * @param bool $inRange
-     * @param \Magento\Framework\View\Result\Page $resultPage
-     * @return \Magento\Framework\View\Result\Page
+     * @param ResultPage $resultPage
+     * @return ResultPage
      */
     protected function setLayoutType($inRange, $resultPage)
     {
diff --git a/app/code/Magento/Cms/Test/Unit/Controller/Page/ViewTest.php b/app/code/Magento/Cms/Test/Unit/Controller/Page/ViewTest.php
@@ -3,73 +3,80 @@
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
+declare(strict_types=1);
+
 namespace Magento\Cms\Test\Unit\Controller\Page;
 
-class ViewTest extends \PHPUnit\Framework\TestCase
+use Magento\Cms\Controller\Page\View;
+use Magento\Cms\Helper\Page as PageHelper;
+use Magento\Framework\App\RequestInterface;
+use Magento\Framework\Controller\Result\Forward;
+use Magento\Framework\Controller\Result\ForwardFactory;
+use Magento\Framework\TestFramework\Unit\Helper\ObjectManager as ObjectManagerHelper;
+use Magento\Framework\View\Result\Page;
+use PHPUnit\Framework\MockObject\MockObject;
+use PHPUnit\Framework\TestCase;
+
+class ViewTest extends TestCase
 {
+    private const STUB_PAGE_ID = 2;
+
     /**
-     * @var \Magento\Cms\Controller\Page\View
+     * @var View
      */
     protected $controller;
 
     /**
-     * @var \PHPUnit_Framework_MockObject_MockObject
+     * @var MockObject
      */
-    protected $cmsHelperMock;
+    protected $pageHelperMock;
 
     /**
-     * @var \PHPUnit_Framework_MockObject_MockObject
+     * @var MockObject|RequestInterface
      */
     protected $requestMock;
 
     /**
-     * @var \Magento\Framework\Controller\Result\ForwardFactory|\PHPUnit_Framework_MockObject_MockObject
+     * @var MockObject|ForwardFactory
      */
     protected $forwardFactoryMock;
 
     /**
-     * @var \Magento\Framework\Controller\Result\Forward|\PHPUnit_Framework_MockObject_MockObject
+     * @var MockObject|Forward
      */
     protected $forwardMock;
 
     /**
-     * @var \Magento\Framework\View\Result\Page|\PHPUnit_Framework_MockObject_MockObject
+     * @var MockObject|Page
      */
     protected $resultPageMock;
 
-    /**
-     * @var string
-     */
-    protected $pageId = '2';
-
     protected function setUp()
     {
-        $helper = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
-        $objectManagerMock = $this->createMock(\Magento\Framework\ObjectManagerInterface::class);
-        $responseMock = $this->createMock(\Magento\Framework\App\Response\Http::class);
-        $this->resultPageMock = $this->getMockBuilder(\Magento\Framework\View\Result\Page::class)
+        $objectManager = new ObjectManagerHelper($this);
+
+        $this->resultPageMock = $this->getMockBuilder(Page::class)
             ->disableOriginalConstructor()
             ->getMock();
-        $this->forwardFactoryMock = $this->getMockBuilder(\Magento\Framework\Controller\Result\ForwardFactory::class)
+        $this->forwardFactoryMock = $this->getMockBuilder(ForwardFactory::class)
             ->setMethods(['create'])
             ->disableOriginalConstructor()
             ->getMock();
-        $this->forwardMock = $this->getMockBuilder(\Magento\Framework\Controller\Result\Forward::class)
+        $this->forwardMock = $this->getMockBuilder(Forward::class)
             ->disableOriginalConstructor()
             ->getMock();
         $this->forwardFactoryMock->expects($this->any())
             ->method('create')
             ->willReturn($this->forwardMock);
 
-        $this->requestMock = $this->createMock(\Magento\Framework\App\Request\Http::class);
-        $this->cmsHelperMock = $this->createMock(\Magento\Cms\Helper\Page::class);
-        $objectManagerMock->expects($this->once())->method('get')->willReturn($this->cmsHelperMock);
-        $this->controller = $helper->getObject(
-            \Magento\Cms\Controller\Page\View::class,
+        $this->requestMock = $this->createMock(RequestInterface::class);
+        $this->pageHelperMock = $this->createMock(PageHelper::class);
+
+        $this->controller = $objectManager->getObject(
+            View::class,
             [
-                'response' => $responseMock,
-                'objectManager' => $objectManagerMock,
                 'request' => $this->requestMock,
+                'pageHelper' => $this->pageHelperMock,
                 'resultForwardFactory' => $this->forwardFactoryMock
             ]
         );
@@ -81,13 +88,13 @@ public function testExecuteResultPage()
             ->method('getParam')
             ->willReturnMap(
                 [
-                    ['page_id', $this->pageId, $this->pageId],
-                    ['id', false, $this->pageId]
+                    ['page_id', null, self::STUB_PAGE_ID],
+                    ['id', null, self::STUB_PAGE_ID]
                 ]
             );
-        $this->cmsHelperMock->expects($this->once())
+        $this->pageHelperMock->expects($this->once())
             ->method('prepareResultPage')
-            ->with($this->controller, $this->pageId)
+            ->with($this->controller, self::STUB_PAGE_ID)
             ->willReturn($this->resultPageMock);
         $this->assertSame($this->resultPageMock, $this->controller->execute());
     }
@@ -98,8 +105,8 @@ public function testExecuteResultForward()
             ->method('getParam')
             ->willReturnMap(
                 [
-                    ['page_id', $this->pageId, $this->pageId],
-                    ['id', false, $this->pageId]
+                    ['page_id', null, self::STUB_PAGE_ID],
+                    ['id', null, self::STUB_PAGE_ID]
                 ]
             );
         $this->forwardMock->expects($this->once())
