Skip to content

Commit c2a38cd

Browse files
sidolovsidolov
authored
ENGCOM-8498: Fatal error page occurs if use wrong shipment id ,invoice id ,creditmemo id passed in url #30410 
2 parents 95be919 + d7f15e9 commit c2a38cd

File tree

16 files changed

+312
-64
lines changed

16 files changed

+312
-64
lines changed

‎app/code/Magento/Sales/Controller/Adminhtml/Order/Creditmemo/View.php‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,9 @@
66
namespace Magento\Sales\Controller\Adminhtml\Order\Creditmemo;
77

88
use Magento\Backend\App\Action;
9+
use Magento\Framework\App\Action\HttpGetActionInterface as HttpGetActionInterface;
910

10-
class View extends \Magento\Backend\App\Action
11+
class View extends \Magento\Backend\App\Action implements HttpGetActionInterface
1112
{
1213
/**
1314
* Authorization level of a basic admin session
@@ -75,9 +76,9 @@ public function execute()
7576
}
7677
return $resultPage;
7778
} else {
78-
$resultForward = $this->resultForwardFactory->create();
79-
$resultForward->forward('noroute');
80-
return $resultForward;
79+
$resultRedirect = $this->resultRedirectFactory->create();
80+
$resultRedirect->setPath('sales/creditmemo');
81+
return $resultRedirect;
8182
}
8283
}
8384
}

‎app/code/Magento/Sales/Controller/Adminhtml/Order/CreditmemoLoader.php‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,12 @@
88

99
use Magento\Framework\DataObject;
1010
use Magento\Sales\Api\CreditmemoRepositoryInterface;
11-
use \Magento\Sales\Model\Order\CreditmemoFactory;
11+
use Magento\Sales\Model\Order;
12+
use Magento\Sales\Model\Order\CreditmemoFactory;
1213

1314
/**
14-
* Class CreditmemoLoader
15+
* Loader for creditmemo
1516
*
16-
* @package Magento\Sales\Controller\Adminhtml\Order
1717
* @method CreditmemoLoader setCreditmemoId($id)
1818
* @method CreditmemoLoader setCreditmemo($creditMemo)
1919
* @method CreditmemoLoader setInvoiceId($id)
@@ -22,6 +22,7 @@
2222
* @method string getCreditmemo()
2323
* @method int getInvoiceId()
2424
* @method int getOrderId()
25+
* @SuppressWarnings(PHPMD.CookieAndSessionMisuse)
2526
*/
2627
class CreditmemoLoader extends DataObject
2728
{
@@ -129,7 +130,8 @@ protected function _getItemData()
129130

130131
/**
131132
* Check if creditmeno can be created for order
132-
* @param \Magento\Sales\Model\Order $order
133+
*
134+
* @param Order $order
133135
* @return bool
134136
*/
135137
protected function _canCreditmemo($order)
@@ -153,7 +155,9 @@ protected function _canCreditmemo($order)
153155
}
154156

155157
/**
156-
* @param \Magento\Sales\Model\Order $order
158+
* Inits invoice
159+
*
160+
* @param Order $order
157161
* @return $this|bool
158162
*/
159163
protected function _initInvoice($order)
@@ -181,7 +185,12 @@ public function load()
181185
$creditmemoId = $this->getCreditmemoId();
182186
$orderId = $this->getOrderId();
183187
if ($creditmemoId) {
184-
$creditmemo = $this->creditmemoRepository->get($creditmemoId);
188+
try {
189+
$creditmemo = $this->creditmemoRepository->get($creditmemoId);
190+
} catch (\Exception $e) {
191+
$this->messageManager->addErrorMessage(__('This creditmemo no longer exists.'));
192+
return false;
193+
}
185194
} elseif ($orderId) {
186195
$data = $this->getCreditmemo();
187196
$order = $this->orderFactory->create()->load($orderId);

‎app/code/Magento/Sales/Controller/Adminhtml/Order/Invoice/View.php‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,10 @@ public function execute()
4444
{
4545
$invoice = $this->getInvoice();
4646
if (!$invoice) {
47-
/** @var \Magento\Framework\Controller\Result\Forward $resultForward */
48-
$resultForward = $this->resultForwardFactory->create();
49-
return $resultForward->forward('noroute');
47+
/** @var \Magento\Framework\Controller\Result\RedirectFactory $resultRedirect */
48+
$resultRedirect = $this->resultRedirectFactory->create();
49+
$resultRedirect->setPath('sales/invoice');
50+
return $resultRedirect;
5051
}
5152

5253
/** @var \Magento\Backend\Model\View\Result\Page $resultPage */

‎app/code/Magento/Sales/Test/Mftf/ActionGroup/AdminGoToCreditmemoViewActionGroup.xml‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!--
3+
/**
4+
* Copyright © Magento, Inc. All rights reserved.
5+
* See COPYING.txt for license details.
6+
*/
7+
-->
8+
9+
<actionGroups xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
10+
xsi:noNamespaceSchemaLocation="urn:magento:mftf:Test/etc/actionGroupSchema.xsd">
11+
<actionGroup name="AdminGoToCreditmemoViewActionGroup">
12+
<annotations>
13+
<description>Goes to the Order Creditmemo View Page.</description>
14+
</annotations>
15+
<arguments>
16+
<argument name="identifier" type="string"/>
17+
</arguments>
18+
19+
<amOnPage url="{{AdminCreditMemoViewPage.url}}{{identifier}}" stepKey="amOnCreditmemoViewPage"/>
20+
<waitForPageLoad stepKey="waitForPageLoad"/>
21+
</actionGroup>
22+
</actionGroups>

‎app/code/Magento/Sales/Test/Mftf/Page/AdminCreditMemoViewPage.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
<pages xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
1010
xsi:noNamespaceSchemaLocation="urn:magento:mftf:Page/etc/PageObject.xsd">
11-
<page name="AdminCreditMemoViewPage" url="sales/order_creditmemo/view/creditmemo_id/{{memoId}}/" parameterized="true" area="admin" module="Magento_Sales">
11+
<page name="AdminCreditMemoViewPage" url="sales/creditmemo/view/creditmemo_id/" area="admin" module="Magento_Sales">
1212
<section name="AdminCreditMemoViewItemsSection"/>
1313
<section name="AdminCreditMemoViewTotalSection"/>
1414
</page>

‎app/code/Magento/Sales/Test/Mftf/Test/AdminOpenCreditmemoViewPageWithWrongCreditmemoIdTest.xml‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!--
3+
/**
4+
* Copyright © Magento, Inc. All rights reserved.
5+
* See COPYING.txt for license details.
6+
*/
7+
-->
8+
9+
<tests xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
10+
xsi:noNamespaceSchemaLocation="urn:magento:mftf:Test/etc/testSchema.xsd">
11+
<test name="AdminOpenCreditmemoViewPageWithWrongCreditmemoIdTest">
12+
<annotations>
13+
<stories value="Creditmemo Page With Wrong Creditmemo Id"/>
14+
<title value="Open Creditmemo View Page with Wrong Creditmemo Id"/>
15+
<description value="Open Creditmemo View Page with Wrong Creditmemo Id."/>
16+
<severity value="BLOCKER"/>
17+
<testCaseId value="MC-39500"/>
18+
<group value="sales"/>
19+
</annotations>
20+
<before>
21+
<actionGroup ref="AdminLoginActionGroup" stepKey="LoginAsAdmin"/>
22+
</before>
23+
<after>
24+
<actionGroup ref="AdminLogoutActionGroup" stepKey="logout"/>
25+
</after>
26+
27+
<actionGroup ref="AdminGoToCreditmemoViewActionGroup" stepKey="navigateOpenCreditmemoViewPage">
28+
<argument name="identifier" value="test"/>
29+
</actionGroup>
30+
31+
<waitForPageLoad stepKey="waitForPageLoad"/>
32+
<seeInCurrentUrl url="{{AdminCreditMemosGridPage.url}}" stepKey="redirectToCreditMemosGridPage"/>
33+
<see selector="{{AdminMessagesSection.error}}" userInput='This creditmemo no longer exists.' stepKey="seeErrorMessage"/>
34+
</test>
35+
</tests>

‎app/code/Magento/Sales/Test/Unit/Controller/Adminhtml/Order/Creditmemo/ViewTest.php‎

Lines changed: 46 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
use Magento\Backend\Model\View\Result\Forward;
1414
use Magento\Backend\Model\View\Result\ForwardFactory;
1515
use Magento\Backend\Model\View\Result\Page;
16+
use Magento\Backend\Model\View\Result\Redirect;
17+
use Magento\Backend\Model\View\Result\RedirectFactory;
1618
use Magento\Framework\App\ActionFlag;
1719
use Magento\Framework\App\Request\Http;
1820
use Magento\Framework\Message\Manager;
@@ -105,6 +107,17 @@ class ViewTest extends TestCase
105107
*/
106108
protected $pageTitleMock;
107109

110+
/**
111+
* @var \Magento\Sales\Controller\Adminhtml\Order\Creditmemo\View
112+
* @var RedirectFactory|MockObject
113+
*/
114+
protected $resultRedirectFactoryMock;
115+
116+
/**
117+
* @var Redirect|MockObject
118+
*/
119+
protected $resultRedirectMock;
120+
108121
/**
109122
* @var PageFactory|MockObject
110123
*/
@@ -130,9 +143,6 @@ class ViewTest extends TestCase
130143
*/
131144
protected function setUp(): void
132145
{
133-
$titleMock = $this->getMockBuilder(\Magento\Framework\App\Action\Title::class)
134-
->disableOriginalConstructor()
135-
->getMock();
136146
$this->invoiceMock = $this->getMockBuilder(Invoice::class)
137147
->disableOriginalConstructor()
138148
->getMock();
@@ -203,7 +213,13 @@ protected function setUp(): void
203213
$this->resultForwardMock = $this->getMockBuilder(Forward::class)
204214
->disableOriginalConstructor()
205215
->getMock();
206-
216+
$this->resultRedirectFactoryMock = $this->getMockBuilder(RedirectFactory::class)
217+
->disableOriginalConstructor()
218+
->setMethods(['create'])
219+
->getMock();
220+
$this->resultRedirectMock = $this->getMockBuilder(Redirect::class)
221+
->disableOriginalConstructor()
222+
->getMock();
207223
$this->contextMock->expects($this->any())
208224
->method('getSession')
209225
->willReturn($this->sessionMock);
@@ -219,9 +235,6 @@ protected function setUp(): void
219235
$this->contextMock->expects($this->any())
220236
->method('getObjectManager')
221237
->willReturn($this->objectManagerMock);
222-
$this->contextMock->expects($this->any())
223-
->method('getTitle')
224-
->willReturn($titleMock);
225238
$this->contextMock->expects($this->any())
226239
->method('getMessageManager')
227240
->willReturn($this->messageManagerMock);
@@ -239,7 +252,8 @@ protected function setUp(): void
239252
'context' => $this->contextMock,
240253
'creditmemoLoader' => $this->loaderMock,
241254
'resultPageFactory' => $this->resultPageFactoryMock,
242-
'resultForwardFactory' => $this->resultForwardFactoryMock
255+
'resultForwardFactory' => $this->resultForwardFactoryMock,
256+
'resultRedirectFactory' => $this->resultRedirectFactoryMock
243257
]
244258
);
245259
}
@@ -252,16 +266,11 @@ public function testExecuteNoCreditMemo()
252266
$this->loaderMock->expects($this->once())
253267
->method('load')
254268
->willReturn(false);
255-
$this->resultForwardFactoryMock->expects($this->once())
256-
->method('create')
257-
->willReturn($this->resultForwardMock);
258-
$this->resultForwardMock->expects($this->once())
259-
->method('forward')
260-
->with('noroute')
261-
->willReturnSelf();
262269

270+
$this->prepareRedirect();
271+
$this->setPath('sales/creditmemo');
263272
$this->assertInstanceOf(
264-
Forward::class,
273+
Redirect::class,
265274
$this->controller->execute()
266275
);
267276
}
@@ -322,4 +331,25 @@ public function executeDataProvider()
322331
[$this->invoiceMock]
323332
];
324333
}
334+
335+
/**
336+
* prepareRedirect
337+
*/
338+
protected function prepareRedirect()
339+
{
340+
$this->resultRedirectFactoryMock->expects($this->once())
341+
->method('create')
342+
->willReturn($this->resultRedirectMock);
343+
}
344+
345+
/**
346+
* @param string $path
347+
* @param array $params
348+
*/
349+
protected function setPath($path, $params = [])
350+
{
351+
$this->resultRedirectMock->expects($this->once())
352+
->method('setPath')
353+
->with($path, $params);
354+
}
325355
}

0 commit comments

Comments
 (0)