ENGCOM-8498: Fatal error page occurs if use wrong shipment id ,invoic…

…e id ,creditmemo id passed in url #30410
magento · Dec 22, 2020 · c2a38cd · c2a38cd
2 parents 95be919 + d7f15e9
commit c2a38cd
Show file tree

Hide file tree

Showing 16 changed files with 312 additions and 64 deletions.
diff --git a/app/code/Magento/Sales/Controller/Adminhtml/Order/Creditmemo/View.php b/app/code/Magento/Sales/Controller/Adminhtml/Order/Creditmemo/View.php
@@ -6,8 +6,9 @@
 namespace Magento\Sales\Controller\Adminhtml\Order\Creditmemo;
 
 use Magento\Backend\App\Action;
+use Magento\Framework\App\Action\HttpGetActionInterface as HttpGetActionInterface;
 
-class View extends \Magento\Backend\App\Action
+class View extends \Magento\Backend\App\Action implements HttpGetActionInterface
 {
     /**
      * Authorization level of a basic admin session
@@ -75,9 +76,9 @@ public function execute()
             }
             return $resultPage;
         } else {
-            $resultForward = $this->resultForwardFactory->create();
-            $resultForward->forward('noroute');
-            return $resultForward;
+            $resultRedirect = $this->resultRedirectFactory->create();
+            $resultRedirect->setPath('sales/creditmemo');
+            return $resultRedirect;
         }
     }
 }
diff --git a/app/code/Magento/Sales/Controller/Adminhtml/Order/CreditmemoLoader.php b/app/code/Magento/Sales/Controller/Adminhtml/Order/CreditmemoLoader.php
@@ -8,12 +8,12 @@
 
 use Magento\Framework\DataObject;
 use Magento\Sales\Api\CreditmemoRepositoryInterface;
-use \Magento\Sales\Model\Order\CreditmemoFactory;
+use Magento\Sales\Model\Order;
+use Magento\Sales\Model\Order\CreditmemoFactory;
 
 /**
- * Class CreditmemoLoader
+ * Loader for creditmemo
  *
- * @package Magento\Sales\Controller\Adminhtml\Order
  * @method CreditmemoLoader setCreditmemoId($id)
  * @method CreditmemoLoader setCreditmemo($creditMemo)
  * @method CreditmemoLoader setInvoiceId($id)
@@ -22,6 +22,7 @@
  * @method string getCreditmemo()
  * @method int getInvoiceId()
  * @method int getOrderId()
+ * @SuppressWarnings(PHPMD.CookieAndSessionMisuse)
  */
 class CreditmemoLoader extends DataObject
 {
@@ -129,7 +130,8 @@ protected function _getItemData()
 
     /**
      * Check if creditmeno can be created for order
-     * @param \Magento\Sales\Model\Order $order
+     *
+     * @param Order $order
      * @return bool
      */
     protected function _canCreditmemo($order)
@@ -153,7 +155,9 @@ protected function _canCreditmemo($order)
     }
 
     /**
-     * @param \Magento\Sales\Model\Order $order
+     * Inits invoice
+     *
+     * @param Order $order
      * @return $this|bool
      */
     protected function _initInvoice($order)
@@ -181,7 +185,12 @@ public function load()
         $creditmemoId = $this->getCreditmemoId();
         $orderId = $this->getOrderId();
         if ($creditmemoId) {
-            $creditmemo = $this->creditmemoRepository->get($creditmemoId);
+            try {
+                $creditmemo = $this->creditmemoRepository->get($creditmemoId);
+            } catch (\Exception $e) {
+                $this->messageManager->addErrorMessage(__('This creditmemo no longer exists.'));
+                return false;
+            }
         } elseif ($orderId) {
             $data = $this->getCreditmemo();
             $order = $this->orderFactory->create()->load($orderId);

diff --git a/app/code/Magento/Sales/Controller/Adminhtml/Order/Invoice/View.php b/app/code/Magento/Sales/Controller/Adminhtml/Order/Invoice/View.php
@@ -44,9 +44,10 @@ public function execute()
     {
         $invoice = $this->getInvoice();
         if (!$invoice) {
-            /** @var \Magento\Framework\Controller\Result\Forward $resultForward */
-            $resultForward = $this->resultForwardFactory->create();
-            return $resultForward->forward('noroute');
+            /** @var \Magento\Framework\Controller\Result\RedirectFactory $resultRedirect */
+            $resultRedirect = $this->resultRedirectFactory->create();
+            $resultRedirect->setPath('sales/invoice');
+            return $resultRedirect;
         }
 
         /** @var \Magento\Backend\Model\View\Result\Page $resultPage */

diff --git a/app/code/Magento/Sales/Test/Mftf/ActionGroup/AdminGoToCreditmemoViewActionGroup.xml b/app/code/Magento/Sales/Test/Mftf/ActionGroup/AdminGoToCreditmemoViewActionGroup.xml
@@ -0,0 +1,22 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ /**
+  * Copyright © Magento, Inc. All rights reserved.
+  * See COPYING.txt for license details.
+  */
+-->
+
+<actionGroups xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+              xsi:noNamespaceSchemaLocation="urn:magento:mftf:Test/etc/actionGroupSchema.xsd">
+    <actionGroup name="AdminGoToCreditmemoViewActionGroup">
+        <annotations>
+            <description>Goes to the Order Creditmemo View Page.</description>
+        </annotations>
+        <arguments>
+            <argument name="identifier" type="string"/>
+        </arguments>
+
+        <amOnPage url="{{AdminCreditMemoViewPage.url}}{{identifier}}" stepKey="amOnCreditmemoViewPage"/>
+        <waitForPageLoad stepKey="waitForPageLoad"/>
+    </actionGroup>
+</actionGroups>
diff --git a/app/code/Magento/Sales/Test/Mftf/Page/AdminCreditMemoViewPage.xml b/app/code/Magento/Sales/Test/Mftf/Page/AdminCreditMemoViewPage.xml
@@ -8,7 +8,7 @@
 
 <pages xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
        xsi:noNamespaceSchemaLocation="urn:magento:mftf:Page/etc/PageObject.xsd">
-    <page name="AdminCreditMemoViewPage" url="sales/order_creditmemo/view/creditmemo_id/{{memoId}}/" parameterized="true" area="admin" module="Magento_Sales">
+    <page name="AdminCreditMemoViewPage" url="sales/creditmemo/view/creditmemo_id/" area="admin" module="Magento_Sales">
         <section name="AdminCreditMemoViewItemsSection"/>
         <section name="AdminCreditMemoViewTotalSection"/>
     </page>

diff --git a/...ode/Magento/Sales/Test/Mftf/Test/AdminOpenCreditmemoViewPageWithWrongCreditmemoIdTest.xml b/...ode/Magento/Sales/Test/Mftf/Test/AdminOpenCreditmemoViewPageWithWrongCreditmemoIdTest.xml
@@ -0,0 +1,35 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ /**
+  * Copyright © Magento, Inc. All rights reserved.
+  * See COPYING.txt for license details.
+  */
+-->
+
+<tests xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+       xsi:noNamespaceSchemaLocation="urn:magento:mftf:Test/etc/testSchema.xsd">
+    <test name="AdminOpenCreditmemoViewPageWithWrongCreditmemoIdTest">
+        <annotations>
+            <stories value="Creditmemo Page With Wrong Creditmemo Id"/>
+            <title value="Open Creditmemo View Page with Wrong Creditmemo Id"/>
+            <description value="Open Creditmemo View Page with Wrong Creditmemo Id."/>
+            <severity value="BLOCKER"/>
+            <testCaseId value="MC-39500"/>
+            <group value="sales"/>
+        </annotations>
+        <before>
+            <actionGroup ref="AdminLoginActionGroup" stepKey="LoginAsAdmin"/>
+        </before>
+        <after>
+            <actionGroup ref="AdminLogoutActionGroup" stepKey="logout"/>
+        </after>
+
+        <actionGroup ref="AdminGoToCreditmemoViewActionGroup" stepKey="navigateOpenCreditmemoViewPage">
+            <argument name="identifier" value="test"/>
+        </actionGroup>
+
+        <waitForPageLoad stepKey="waitForPageLoad"/>
+        <seeInCurrentUrl url="{{AdminCreditMemosGridPage.url}}" stepKey="redirectToCreditMemosGridPage"/>
+        <see selector="{{AdminMessagesSection.error}}" userInput='This creditmemo no longer exists.' stepKey="seeErrorMessage"/>
+    </test>
+</tests>
diff --git a/app/code/Magento/Sales/Test/Unit/Controller/Adminhtml/Order/Creditmemo/ViewTest.php b/app/code/Magento/Sales/Test/Unit/Controller/Adminhtml/Order/Creditmemo/ViewTest.php
@@ -13,6 +13,8 @@
 use Magento\Backend\Model\View\Result\Forward;
 use Magento\Backend\Model\View\Result\ForwardFactory;
 use Magento\Backend\Model\View\Result\Page;
+use Magento\Backend\Model\View\Result\Redirect;
+use Magento\Backend\Model\View\Result\RedirectFactory;
 use Magento\Framework\App\ActionFlag;
 use Magento\Framework\App\Request\Http;
 use Magento\Framework\Message\Manager;
@@ -105,6 +107,17 @@ class ViewTest extends TestCase
      */
     protected $pageTitleMock;
 
+    /**
+     * @var \Magento\Sales\Controller\Adminhtml\Order\Creditmemo\View
+     * @var RedirectFactory|MockObject
+     */
+    protected $resultRedirectFactoryMock;
+
+    /**
+     * @var Redirect|MockObject
+     */
+    protected $resultRedirectMock;
+
     /**
      * @var PageFactory|MockObject
      */
@@ -130,9 +143,6 @@ class ViewTest extends TestCase
      */
     protected function setUp(): void
     {
-        $titleMock = $this->getMockBuilder(\Magento\Framework\App\Action\Title::class)
-            ->disableOriginalConstructor()
-            ->getMock();
         $this->invoiceMock = $this->getMockBuilder(Invoice::class)
             ->disableOriginalConstructor()
             ->getMock();
@@ -203,7 +213,13 @@ protected function setUp(): void
         $this->resultForwardMock = $this->getMockBuilder(Forward::class)
             ->disableOriginalConstructor()
             ->getMock();
-
+        $this->resultRedirectFactoryMock = $this->getMockBuilder(RedirectFactory::class)
+            ->disableOriginalConstructor()
+            ->setMethods(['create'])
+            ->getMock();
+        $this->resultRedirectMock = $this->getMockBuilder(Redirect::class)
+            ->disableOriginalConstructor()
+            ->getMock();
         $this->contextMock->expects($this->any())
             ->method('getSession')
             ->willReturn($this->sessionMock);
@@ -219,9 +235,6 @@ protected function setUp(): void
         $this->contextMock->expects($this->any())
             ->method('getObjectManager')
             ->willReturn($this->objectManagerMock);
-        $this->contextMock->expects($this->any())
-            ->method('getTitle')
-            ->willReturn($titleMock);
         $this->contextMock->expects($this->any())
             ->method('getMessageManager')
             ->willReturn($this->messageManagerMock);
@@ -239,7 +252,8 @@ protected function setUp(): void
                 'context' => $this->contextMock,
                 'creditmemoLoader' => $this->loaderMock,
                 'resultPageFactory' => $this->resultPageFactoryMock,
-                'resultForwardFactory' => $this->resultForwardFactoryMock
+                'resultForwardFactory' => $this->resultForwardFactoryMock,
+                'resultRedirectFactory' => $this->resultRedirectFactoryMock
             ]
         );
     }
@@ -252,16 +266,11 @@ public function testExecuteNoCreditMemo()
         $this->loaderMock->expects($this->once())
             ->method('load')
             ->willReturn(false);
-        $this->resultForwardFactoryMock->expects($this->once())
-            ->method('create')
-            ->willReturn($this->resultForwardMock);
-        $this->resultForwardMock->expects($this->once())
-            ->method('forward')
-            ->with('noroute')
-            ->willReturnSelf();
 
+        $this->prepareRedirect();
+        $this->setPath('sales/creditmemo');
         $this->assertInstanceOf(
-            Forward::class,
+            Redirect::class,
             $this->controller->execute()
         );
     }
@@ -322,4 +331,25 @@ public function executeDataProvider()
             [$this->invoiceMock]
         ];
     }
+
+    /**
+     * prepareRedirect
+     */
+    protected function prepareRedirect()
+    {
+        $this->resultRedirectFactoryMock->expects($this->once())
+            ->method('create')
+            ->willReturn($this->resultRedirectMock);
+    }
+
+    /**
+     * @param string $path
+     * @param array $params
+     */
+    protected function setPath($path, $params = [])
+    {
+        $this->resultRedirectMock->expects($this->once())
+            ->method('setPath')
+            ->with($path, $params);
+    }
 }