Skip to content

Commit 8e25924

Browse files
author
Oleksii Korshenko
authored
Merge pull request #292 from magento-fearless-kiwis/FearlessKiwis-MAGETWO-57580-Backport-Csrf-delete-the-customer-addresses
Fixed issues: - MAGETWO-57580: [Backport] CSRF delete the customer addresses - MAGETWO-56963: Clone Clone [GitHub] State/Province field doesn't show as required on the add new address page. #5279
2 parents a93ea30 + 9fb1287 commit 8e25924

File tree

5 files changed

+203
-13
lines changed

5 files changed

+203
-13
lines changed

app/code/Magento/Checkout/view/frontend/web/js/region-updater.js

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ define([
2525
isMultipleCountriesAllowed: true
2626
},
2727

28+
/**
29+
*
30+
* @private
31+
*/
2832
_create: function () {
2933
this._initCountryElement();
3034

@@ -43,12 +47,18 @@ define([
4347
}, this));
4448
},
4549

46-
_initCountryElement: function() {
50+
/**
51+
*
52+
* @private
53+
*/
54+
_initCountryElement: function () {
55+
4756
if (this.options.isMultipleCountriesAllowed) {
4857
this.element.parents('div.field').show();
4958
this.element.on('change', $.proxy(function (e) {
5059
this._updateRegion($(e.target).val());
5160
}, this));
61+
5262
if (this.options.isCountryRequired) {
5363
this.element.addClass('required-entry');
5464
this.element.parents('div.field').addClass('required');
@@ -60,6 +70,7 @@ define([
6070

6171
/**
6272
* Remove options from dropdown list
73+
*
6374
* @param {Object} selectElement - jQuery object for dropdown list
6475
* @private
6576
*/
@@ -113,7 +124,7 @@ define([
113124
* @private
114125
*/
115126
_clearError: function () {
116-
if (this.options.clearError && typeof (this.options.clearError) === 'function') {
127+
if (this.options.clearError && typeof this.options.clearError === 'function') {
117128
this.options.clearError.call(this);
118129
} else {
119130
if (!this.options.form) {
@@ -122,12 +133,19 @@ define([
122133

123134
this.options.form = $(this.options.form);
124135

125-
this.options.form && this.options.form.data('validation') && this.options.form.validation('clearError',
136+
this.options.form && this.options.form.data('validator') && this.options.form.validation('clearError',
126137
this.options.regionListId, this.options.regionInputId, this.options.postcodeId);
138+
139+
// Clean up errors on region & zip fix
140+
$(this.options.regionInputId).removeClass('mage-error').parent().find('[generated]').remove();
141+
$(this.options.regionListId).removeClass('mage-error').parent().find('[generated]').remove();
142+
$(this.options.postcodeId).removeClass('mage-error').parent().find('[generated]').remove();
127143
}
128144
},
145+
129146
/**
130147
* Update dropdown list based on the country selected
148+
*
131149
* @param {String} country - 2 uppercase letter for country code
132150
* @private
133151
*/
@@ -182,11 +200,12 @@ define([
182200
if (!this.options.optionalRegionAllowed) {
183201
regionInput.attr('disabled', 'disabled');
184202
}
203+
requiredLabel.removeClass('required');
204+
regionInput.removeClass('required-entry');
185205
}
186206

187207
regionList.removeClass('required-entry').hide();
188208
regionInput.show();
189-
requiredLabel.removeClass('required');
190209
label.attr('for', regionInput.attr('id'));
191210
}
192211

@@ -208,10 +227,11 @@ define([
208227
* @private
209228
*/
210229
_checkRegionRequired: function (country) {
211-
this.options.isRegionRequired = false;
212230
var self = this;
231+
232+
this.options.isRegionRequired = false;
213233
$.each(this.options.regionJson.config.regions_required, function (index, elem) {
214-
if (elem == country) {
234+
if (elem === country) {
215235
self.options.isRegionRequired = true;
216236
}
217237
});

app/code/Magento/Customer/Controller/Address/Delete.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ public function execute()
1515
{
1616
$addressId = $this->getRequest()->getParam('id', false);
1717

18-
if ($addressId) {
18+
if ($addressId && $this->_formKeyValidator->validate($this->getRequest())) {
1919
try {
2020
$address = $this->_addressRepository->getById($addressId);
2121
if ($address->getCustomerId() === $this->_getSession()->getCustomerId()) {
Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
<?php
2+
/**
3+
* Copyright © 2016 Magento. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
namespace Magento\Customer\Test\Unit\Controller\Address;
7+
8+
use Magento\Customer\Controller\Address\Delete;
9+
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager as ObjectManagerHelper;
10+
11+
/**
12+
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
13+
*/
14+
class DeleteTest extends \PHPUnit_Framework_TestCase
15+
{
16+
/** @var Delete */
17+
protected $model;
18+
19+
/** @var \Magento\Customer\Model\Session|\PHPUnit_Framework_MockObject_MockObject */
20+
protected $sessionMock;
21+
22+
/** @var \Magento\Framework\Data\Form\FormKey\Validator|\PHPUnit_Framework_MockObject_MockObject */
23+
protected $validatorMock;
24+
25+
/** @var \Magento\Customer\Api\AddressRepositoryInterface|\PHPUnit_Framework_MockObject_MockObject */
26+
protected $addressRepositoryMock;
27+
28+
/** @var \Magento\Framework\App\RequestInterface|\PHPUnit_Framework_MockObject_MockObject */
29+
protected $request;
30+
31+
/** @var \Magento\Customer\Api\Data\AddressInterface|\PHPUnit_Framework_MockObject_MockObject */
32+
protected $address;
33+
34+
/** @var \Magento\Framework\Message\ManagerInterface|\PHPUnit_Framework_MockObject_MockObject */
35+
protected $messageManager;
36+
37+
/** @var \Magento\Framework\Controller\Result\RedirectFactory|\PHPUnit_Framework_MockObject_MockObject */
38+
protected $resultRedirectFactory;
39+
40+
/** @var \Magento\Framework\Controller\Result\Redirect|\PHPUnit_Framework_MockObject_MockObject */
41+
protected $resultRedirect;
42+
43+
protected function setUp()
44+
{
45+
$this->sessionMock = $this->getMockBuilder(\Magento\Customer\Model\Session::class)
46+
->disableOriginalConstructor()
47+
->getMock();
48+
$this->validatorMock = $this->getMockBuilder(\Magento\Framework\Data\Form\FormKey\Validator::class)
49+
->disableOriginalConstructor()
50+
->getMock();
51+
$this->addressRepositoryMock = $this->getMockBuilder(\Magento\Customer\Api\AddressRepositoryInterface::class)
52+
->getMockForAbstractClass();
53+
$this->request = $this->getMockBuilder(\Magento\Framework\App\RequestInterface::class)
54+
->getMockForAbstractClass();
55+
$this->address = $this->getMockBuilder(\Magento\Customer\Api\Data\AddressInterface::class)
56+
->getMockForAbstractClass();
57+
$this->messageManager = $this->getMockBuilder(\Magento\Framework\Message\ManagerInterface::class)
58+
->getMockForAbstractClass();
59+
$this->resultRedirectFactory =
60+
$this->getMockBuilder(\Magento\Framework\Controller\Result\RedirectFactory::class)
61+
->disableOriginalConstructor()
62+
->getMock();
63+
$this->resultRedirect = $this->getMockBuilder(\Magento\Framework\Controller\Result\Redirect::class)
64+
->disableOriginalConstructor()
65+
->getMock();
66+
$this->request = $this->getMockBuilder(\Magento\Framework\App\RequestInterface::class)
67+
->getMockForAbstractClass();
68+
69+
$objectManager = new ObjectManagerHelper($this);
70+
$this->model = $objectManager->getObject(
71+
Delete::class,
72+
[
73+
'request' => $this->request,
74+
'resultRedirectFactory' => $this->resultRedirectFactory,
75+
'messageManager' => $this->messageManager,
76+
'customerSession' => $this->sessionMock,
77+
'formKeyValidator' => $this->validatorMock,
78+
'addressRepository' => $this->addressRepositoryMock
79+
]
80+
);
81+
}
82+
83+
public function testExecute()
84+
{
85+
$addressId = 1;
86+
$customerId = 2;
87+
88+
$this->resultRedirectFactory->expects($this->once())
89+
->method('create')
90+
->willReturn($this->resultRedirect);
91+
$this->request->expects($this->once())
92+
->method('getParam')
93+
->with('id', false)
94+
->willReturn($addressId);
95+
$this->validatorMock->expects($this->once())
96+
->method('validate')
97+
->with($this->request)
98+
->willReturn(true);
99+
$this->addressRepositoryMock->expects($this->once())
100+
->method('getById')
101+
->with($addressId)
102+
->willReturn($this->address);
103+
$this->sessionMock->expects($this->once())
104+
->method('getCustomerId')
105+
->willReturn($customerId);
106+
$this->address->expects($this->once())
107+
->method('getCustomerId')
108+
->willReturn($customerId);
109+
$this->addressRepositoryMock->expects($this->once())
110+
->method('deleteById')
111+
->with($addressId);
112+
$this->messageManager->expects($this->once())
113+
->method('addSuccess')
114+
->with(__('You deleted the address.'));
115+
$this->resultRedirect->expects($this->once())
116+
->method('setPath')
117+
->with('*/*/index')
118+
->willReturnSelf();
119+
$this->assertSame($this->resultRedirect, $this->model->execute());
120+
}
121+
122+
public function testExecuteWithException()
123+
{
124+
$addressId = 1;
125+
$customerId = 2;
126+
127+
$this->resultRedirectFactory->expects($this->once())
128+
->method('create')
129+
->willReturn($this->resultRedirect);
130+
$this->request->expects($this->once())
131+
->method('getParam')
132+
->with('id', false)
133+
->willReturn($addressId);
134+
$this->validatorMock->expects($this->once())
135+
->method('validate')
136+
->with($this->request)
137+
->willReturn(true);
138+
$this->addressRepositoryMock->expects($this->once())
139+
->method('getById')
140+
->with($addressId)
141+
->willReturn($this->address);
142+
$this->sessionMock->expects($this->once())
143+
->method('getCustomerId')
144+
->willReturn($customerId);
145+
$this->address->expects($this->once())
146+
->method('getCustomerId')
147+
->willReturn(34);
148+
$exception = new \Exception('Exception');
149+
$this->messageManager->expects($this->once())
150+
->method('addError')
151+
->with(__('We can\'t delete the address right now.'))
152+
->willThrowException($exception);
153+
$this->messageManager->expects($this->once())
154+
->method('addException')
155+
->with($exception, __('We can\'t delete the address right now.'));
156+
$this->resultRedirect->expects($this->once())
157+
->method('setPath')
158+
->with('*/*/index')
159+
->willReturnSelf();
160+
$this->assertSame($this->resultRedirect, $this->model->execute());
161+
}
162+
}

app/code/Magento/Customer/view/frontend/web/address.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,12 @@ define([
6161
actions: {
6262
confirm: function() {
6363
if (typeof $(e.target).parent().data('address') !== 'undefined') {
64-
window.location = self.options.deleteUrlPrefix + $(e.target).parent().data('address');
64+
window.location = self.options.deleteUrlPrefix + $(e.target).parent().data('address')
65+
+ '/form_key/' + $.mage.cookies.get('form_key');
6566
}
6667
else {
67-
window.location = self.options.deleteUrlPrefix + $(e.target).data('address');
68+
window.location = self.options.deleteUrlPrefix + $(e.target).data('address')
69+
+ '/form_key/' + $.mage.cookies.get('form_key');
6870
}
6971
}
7072
}

dev/tests/integration/testsuite/Magento/Customer/Controller/AddressTest.php

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,18 @@
55
*/
66
namespace Magento\Customer\Controller;
77

8+
use Magento\Customer\Api\AccountManagementInterface;
9+
use Magento\Framework\Data\Form\FormKey;
810
use Magento\TestFramework\Helper\Bootstrap;
911

1012
class AddressTest extends \Magento\TestFramework\TestCase\AbstractController
1113
{
12-
/** @var \Magento\Customer\Api\AccountManagementInterface */
14+
/** @var AccountManagementInterface */
1315
private $accountManagement;
1416

17+
/** @var FormKey */
18+
private $formKey;
19+
1520
protected function setUp()
1621
{
1722
parent::setUp();
@@ -20,9 +25,8 @@ protected function setUp()
2025
'Magento\Customer\Model\Session',
2126
[$logger]
2227
);
23-
$this->accountManagement = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create(
24-
'Magento\Customer\Api\AccountManagementInterface'
25-
);
28+
$this->accountManagement = Bootstrap::getObjectManager()->create(AccountManagementInterface::class);
29+
$this->formKey = Bootstrap::getObjectManager()->create(FormKey::class);
2630
$customer = $this->accountManagement->authenticate('customer@example.com', 'password');
2731
$session->setCustomerDataAsLoggedIn($customer);
2832
}
@@ -152,6 +156,7 @@ public function testFailedFormPostAction()
152156
public function testDeleteAction()
153157
{
154158
$this->getRequest()->setParam('id', 1);
159+
$this->getRequest()->setParam('form_key', $this->formKey->getFormKey());
155160
// we are overwriting the address coming from the fixture
156161
$this->dispatch('customer/address/delete');
157162

@@ -169,6 +174,7 @@ public function testDeleteAction()
169174
public function testWrongAddressDeleteAction()
170175
{
171176
$this->getRequest()->setParam('id', 555);
177+
$this->getRequest()->setParam('form_key', $this->formKey->getFormKey());
172178
// we are overwriting the address coming from the fixture
173179
$this->dispatch('customer/address/delete');
174180

0 commit comments

Comments
 (0)