Skip to content

Commit 8f08b99

Browse files
author
Joan He
committed
Merge remote-tracking branch 'owls/MAGETWO-96009-comment-email-acl' into BugFixPR
2 parents cf69967 + 165cd6b commit 8f08b99

File tree

2 files changed

+192
-6
lines changed

2 files changed

+192
-6
lines changed

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

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,17 @@
11
<?php
22
/**
3-
*
43
* Copyright © Magento, Inc. All rights reserved.
54
* See COPYING.txt for license details.
65
*/
7-
86
namespace Magento\Sales\Controller\Adminhtml\Order;
97

10-
use Magento\Backend\App\Action;
8+
use Magento\Framework\App\Action\HttpPostActionInterface;
119
use Magento\Sales\Model\Order\Email\Sender\OrderCommentSender;
1210

13-
class AddComment extends \Magento\Sales\Controller\Adminhtml\Order
11+
/**
12+
* Class AddComment
13+
*/
14+
class AddComment extends \Magento\Sales\Controller\Adminhtml\Order implements HttpPostActionInterface
1415
{
1516
/**
1617
* Authorization level of a basic admin session
@@ -19,6 +20,11 @@ class AddComment extends \Magento\Sales\Controller\Adminhtml\Order
1920
*/
2021
const ADMIN_RESOURCE = 'Magento_Sales::comment';
2122

23+
/**
24+
* ACL resource needed to send comment email notification
25+
*/
26+
const ADMIN_SALES_EMAIL_RESOURCE = 'Magento_Sales::emails';
27+
2228
/**
2329
* Add order comment action
2430
*
@@ -36,8 +42,12 @@ public function execute()
3642
);
3743
}
3844

39-
$notify = isset($data['is_customer_notified']) ? $data['is_customer_notified'] : false;
40-
$visible = isset($data['is_visible_on_front']) ? $data['is_visible_on_front'] : false;
45+
$notify = $data['is_customer_notified'] ?? false;
46+
$visible = $data['is_visible_on_front'] ?? false;
47+
48+
if ($notify && !$this->_authorization->isAllowed(self::ADMIN_SALES_EMAIL_RESOURCE)) {
49+
$notify = false;
50+
}
4151

4252
$history = $order->addStatusHistoryComment($data['comment'], $data['status']);
4353
$history->setIsVisibleOnFront($visible);

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

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
namespace Magento\Sales\Test\Unit\Controller\Adminhtml\Order;
7+
8+
class AddCommentTest extends \PHPUnit\Framework\TestCase
9+
{
10+
/**
11+
* @var \Magento\Sales\Controller\Adminhtml\Order\AddComment
12+
*/
13+
private $addCommentController;
14+
15+
/**
16+
* @var \Magento\Backend\App\Action\Context|\PHPUnit_Framework_MockObject_MockObject
17+
*/
18+
private $contextMock;
19+
20+
/**
21+
* @var \Magento\Sales\Model\Order|\PHPUnit_Framework_MockObject_MockObject
22+
*/
23+
private $orderMock;
24+
25+
/**
26+
* @var \Magento\Backend\Model\View\Result\RedirectFactory|\PHPUnit_Framework_MockObject_MockObject
27+
*/
28+
private $resultRedirectFactoryMock;
29+
30+
/**
31+
* @var \Magento\Backend\Model\View\Result\Redirect|\PHPUnit_Framework_MockObject_MockObject
32+
*/
33+
private $resultRedirectMock;
34+
35+
/**
36+
* @var \Magento\Framework\App\Request\Http|\PHPUnit_Framework_MockObject_MockObject
37+
*/
38+
private $requestMock;
39+
40+
/**
41+
* @var \Magento\Sales\Api\OrderRepositoryInterface|\PHPUnit_Framework_MockObject_MockObject
42+
*/
43+
private $orderRepositoryMock;
44+
45+
/**
46+
* @var \Magento\Framework\AuthorizationInterface|\PHPUnit_Framework_MockObject_MockObject
47+
*/
48+
private $authorizationMock;
49+
50+
/**
51+
* @var \Magento\Sales\Model\Order\Status\History|\PHPUnit_Framework_MockObject_MockObject
52+
*/
53+
private $statusHistoryCommentMock;
54+
55+
/**
56+
* @var \Magento\Framework\ObjectManagerInterface|\PHPUnit_Framework_MockObject_MockObject
57+
*/
58+
private $objectManagerMock;
59+
60+
/**
61+
* Test setup
62+
*/
63+
protected function setUp()
64+
{
65+
$this->contextMock = $this->createMock(\Magento\Backend\App\Action\Context::class);
66+
$this->requestMock = $this->createMock(\Magento\Framework\App\Request\Http::class);
67+
$this->orderRepositoryMock = $this->createMock(\Magento\Sales\Api\OrderRepositoryInterface::class);
68+
$this->orderMock = $this->createMock(\Magento\Sales\Model\Order::class);
69+
$this->resultRedirectFactoryMock = $this->createMock(\Magento\Backend\Model\View\Result\RedirectFactory::class);
70+
$this->resultRedirectMock = $this->createMock(\Magento\Backend\Model\View\Result\Redirect::class);
71+
$this->authorizationMock = $this->createMock(\Magento\Framework\AuthorizationInterface::class);
72+
$this->statusHistoryCommentMock = $this->createMock(\Magento\Sales\Model\Order\Status\History::class);
73+
$this->objectManagerMock = $this->createMock(\Magento\Framework\ObjectManagerInterface::class);
74+
75+
$this->contextMock->expects($this->once())->method('getRequest')->willReturn($this->requestMock);
76+
77+
$objectManagerHelper = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
78+
$this->addCommentController = $objectManagerHelper->getObject(
79+
\Magento\Sales\Controller\Adminhtml\Order\AddComment::class,
80+
[
81+
'context' => $this->contextMock,
82+
'orderRepository' => $this->orderRepositoryMock,
83+
'_authorization' => $this->authorizationMock,
84+
'_objectManager' => $this->objectManagerMock
85+
]
86+
);
87+
}
88+
89+
/**
90+
* @param array $historyData
91+
* @param bool $userHasResource
92+
* @param bool $expectedNotify
93+
*
94+
* @dataProvider executeWillNotifyCustomerDataProvider
95+
*/
96+
public function testExecuteWillNotifyCustomer(array $historyData, bool $userHasResource, bool $expectedNotify)
97+
{
98+
$orderId = 30;
99+
$this->requestMock->expects($this->once())->method('getParam')->with('order_id')->willReturn($orderId);
100+
$this->orderRepositoryMock->expects($this->once())
101+
->method('get')
102+
->willReturn($this->orderMock);
103+
$this->requestMock->expects($this->once())->method('getPost')->with('history')->willReturn($historyData);
104+
$this->authorizationMock->expects($this->any())->method('isAllowed')->willReturn($userHasResource);
105+
$this->orderMock->expects($this->once())
106+
->method('addStatusHistoryComment')
107+
->willReturn($this->statusHistoryCommentMock);
108+
$this->statusHistoryCommentMock->expects($this->once())->method('setIsCustomerNotified')->with($expectedNotify);
109+
$this->objectManagerMock->expects($this->once())->method('create')->willReturn(
110+
$this->createMock(\Magento\Sales\Model\Order\Email\Sender\OrderCommentSender::class)
111+
);
112+
113+
$this->addCommentController->execute();
114+
}
115+
116+
/**
117+
* @return array
118+
*/
119+
public function executeWillNotifyCustomerDataProvider()
120+
{
121+
return [
122+
'User Has Access - Notify True' => [
123+
'postData' => [
124+
'comment' => 'Great Product!',
125+
'is_customer_notified' => true,
126+
'status' => 'Processing'
127+
],
128+
'userHasResource' => true,
129+
'expectedNotify' => true
130+
],
131+
'User Has Access - Notify False' => [
132+
'postData' => [
133+
'comment' => 'Great Product!',
134+
'is_customer_notified' => false,
135+
'status' => 'Processing'
136+
],
137+
'userHasResource' => true,
138+
'expectedNotify' => false
139+
],
140+
'User Has Access - Notify Unset' => [
141+
'postData' => [
142+
'comment' => 'Great Product!',
143+
'status' => 'Processing'
144+
],
145+
'userHasResource' => true,
146+
'expectedNotify' => false
147+
],
148+
'User No Access - Notify True' => [
149+
'postData' => [
150+
'comment' => 'Great Product!',
151+
'is_customer_notified' => true,
152+
'status' => 'Processing'
153+
],
154+
'userHasResource' => false,
155+
'expectedNotify' => false
156+
],
157+
'User No Access - Notify False' => [
158+
'postData' => [
159+
'comment' => 'Great Product!',
160+
'is_customer_notified' => false,
161+
'status' => 'Processing'
162+
],
163+
'userHasResource' => false,
164+
'expectedNotify' => false
165+
],
166+
'User No Access - Notify Unset' => [
167+
'postData' => [
168+
'comment' => 'Great Product!',
169+
'status' => 'Processing'
170+
],
171+
'userHasResource' => false,
172+
'expectedNotify' => false
173+
],
174+
];
175+
}
176+
}

0 commit comments

Comments
 (0)