Skip to content

Commit d7f15e9

Browse files
magento-engcom-teammagento-engcom-team
committed
ENGCOM-8498: Fatal error page occurs if use wrong shipment id ,invoice id ,creditmemo id passed in url #30410
- Merge Pull Request #30410 from GovindaSharma/magento2:dcmm2020 - Merged commits: 1. ff1a003 2. 0e78fe4 3. ff5de32 4. 1dab430 5. 6b1837b 6. 589b1d2 7. 4ba689a 8. 9e99f43 9. 6359ac5 10. ade5168 11. 1f0bcbf 12. 937f9e5 13. 2221a8b 14. 32ae2ff 15. 3fb5db4 16. e38122a 17. 3b0e854 18. 7006a96 19. beac203 20. 564a8a6 21. ffc0ab7 22. 247817a 23. 5ec7bd4 24. 52b5270
2 parents 05a3e7e + 52b5270 commit d7f15e9

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)