Skip to content

Commit 9e5c194

Browse files
nathanjosiahnathanjosiah
authored
Merge pull request #4870 from magento-borg/borg-2.3.4
[borg] Bug fixes
2 parents 267db35 + d29e805 commit 9e5c194

File tree

8 files changed

+96
-32
lines changed

8 files changed

+96
-32
lines changed

‎app/code/Magento/Config/Test/Unit/Block/System/Config/Form/Field/ImageTest.php

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
*/
66

77
/**
8-
* Tests for \Magento\Framework\Data\Form\Field\Image
8+
* Test for \Magento\Framework\Data\Form\Field\Image.
99
*/
1010
namespace Magento\Config\Test\Unit\Block\System\Config\Form\Field;
1111

@@ -26,15 +26,24 @@ class ImageTest extends \PHPUnit\Framework\TestCase
2626
*/
2727
protected $testData;
2828

29+
/**
30+
* @var \Magento\Framework\Escaper|\PHPUnit\Framework\MockObject\MockObject
31+
*/
32+
private $escaperMock;
33+
34+
/**
35+
* @inheritdoc
36+
*/
2937
protected function setUp()
3038
{
3139
$objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
3240
$this->urlBuilderMock = $this->createMock(\Magento\Framework\Url::class);
41+
$this->escaperMock = $this->createMock(\Magento\Framework\Escaper::class);
3342
$this->image = $objectManager->getObject(
3443
\Magento\Config\Block\System\Config\Form\Field\Image::class,
3544
[
3645
'urlBuilder' => $this->urlBuilderMock,
37-
'_escaper' => $objectManager->getObject(\Magento\Framework\Escaper::class)
46+
'_escaper' => $this->escaperMock,
3847
]
3948
);
4049

@@ -53,6 +62,8 @@ protected function setUp()
5362
}
5463

5564
/**
65+
* Get element with value and check data.
66+
*
5667
* @covers \Magento\Config\Block\System\Config\Form\Field\Image::_getUrl
5768
*/
5869
public function testGetElementHtmlWithValue()
@@ -93,7 +104,17 @@ public function testGetElementHtmlWithValue()
93104
. $this->testData['html_id']
94105
. $this->testData['html_id_suffix'];
95106

107+
$this->escaperMock->expects($this->once())
108+
->method('escapeUrl')
109+
->with($url . $this->testData['path'] . '/' . $this->testData['value'])
110+
->willReturn($url . $this->testData['path'] . '/' . $this->testData['value']);
111+
$this->escaperMock->expects($this->exactly(3))
112+
->method('escapeHtmlAttr')
113+
->with($this->testData['value'])
114+
->willReturn($this->testData['value']);
115+
$this->escaperMock->expects($this->atLeastOnce())->method('escapeHtml')->willReturn($expectedHtmlId);
96116
$html = $this->image->getElementHtml();
117+
97118
$this->assertContains('class="input-file"', $html);
98119
$this->assertContains('<input', $html);
99120
$this->assertContains('type="file"', $html);

‎app/code/Magento/CustomerImportExport/Model/Export/Customer.php

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,9 @@ class Customer extends \Magento\ImportExport\Model\Export\Entity\AbstractEav
2727

2828
const COLUMN_STORE = '_store';
2929

30-
/**
31-
* Attribute collection name
30+
/** Attribute collection name.
31+
*
32+
* Used to resolve entity attribute collection.
3233
*/
3334
const ATTRIBUTE_COLLECTION_NAME = \Magento\Customer\Model\ResourceModel\Attribute\Collection::class;
3435

@@ -139,12 +140,12 @@ protected function _getEntityCollection()
139140
}
140141

141142
/**
142-
* {@inheritdoc}
143+
* @inheritdoc
143144
*/
144145
protected function _getHeaderColumns()
145146
{
146147
$validAttributeCodes = $this->_getExportAttributeCodes();
147-
return array_merge($this->_permanentAttributes, $validAttributeCodes, ['password']);
148+
return array_merge($this->_permanentAttributes, $validAttributeCodes);
148149
}
149150

150151
/**

‎app/code/Magento/User/Controller/Adminhtml/User/Delete.php

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
<?php
22
/**
3-
*
43
* Copyright © Magento, Inc. All rights reserved.
54
* See COPYING.txt for license details.
65
*/
@@ -9,10 +8,16 @@
98

109
use Magento\User\Block\User\Edit\Tab\Main as UserEdit;
1110
use Magento\Framework\Exception\AuthenticationException;
11+
use Magento\Framework\App\Action\HttpPostActionInterface as HttpPostActionInterface;
1212

13-
class Delete extends \Magento\User\Controller\Adminhtml\User
13+
/**
14+
* Delete admin user.
15+
*/
16+
class Delete extends \Magento\User\Controller\Adminhtml\User implements HttpPostActionInterface
1417
{
1518
/**
19+
* Execute delete action.
20+
*
1621
* @return void
1722
*/
1823
public function execute()
@@ -37,7 +42,7 @@ public function execute()
3742
$currentUser->performIdentityCheck($currentUserPassword);
3843
/** @var \Magento\User\Model\User $model */
3944
$model = $this->_userFactory->create();
40-
$model->setId($userId);
45+
$model->load($userId);
4146
$model->delete();
4247
$this->messageManager->addSuccess(__('You deleted the user.'));
4348
$this->_redirect('adminhtml/*/');

‎app/code/Magento/User/Model/ResourceModel/User.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,7 @@ protected function _afterLoad(\Magento\Framework\Model\AbstractModel $user)
253253
*/
254254
public function delete(\Magento\Framework\Model\AbstractModel $user)
255255
{
256+
$user->beforeDelete();
256257
$this->_beforeDelete($user);
257258
$connection = $this->getConnection();
258259

@@ -268,7 +269,9 @@ public function delete(\Magento\Framework\Model\AbstractModel $user)
268269
$connection->rollBack();
269270
return false;
270271
}
272+
$user->afterDelete();
271273
$connection->commit();
274+
$user->afterDeleteCommit();
272275
$this->_afterDelete($user);
273276
return true;
274277
}

‎app/code/Magento/User/Test/Unit/Controller/Adminhtml/User/DeleteTest.php

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ protected function setUp()
8181

8282
$this->userMock = $this->getMockBuilder(\Magento\User\Model\User::class)
8383
->disableOriginalConstructor()
84-
->setMethods(['getId', 'performIdentityCheck', 'delete'])
84+
->setMethods(['getId', 'performIdentityCheck', 'delete', 'load'])
8585
->getMock();
8686

8787
$this->userFactoryMock = $this->getMockBuilder(\Magento\User\Model\UserFactory::class)
@@ -134,15 +134,18 @@ public function testExecute($currentUserPassword, $userId, $currentUserId, $resu
134134

135135
$this->requestMock->expects($this->any())
136136
->method('getPost')
137-
->willReturnMap([
138-
['user_id', $userId],
139-
[\Magento\User\Block\User\Edit\Tab\Main::CURRENT_USER_PASSWORD_FIELD, $currentUserPassword],
140-
]);
137+
->willReturnMap(
138+
[
139+
['user_id', $userId],
140+
[\Magento\User\Block\User\Edit\Tab\Main::CURRENT_USER_PASSWORD_FIELD, $currentUserPassword],
141+
]
142+
);
141143

142144
$userMock = clone $currentUserMock;
143145

144146
$this->userFactoryMock->expects($this->any())->method('create')->will($this->returnValue($userMock));
145147
$this->responseMock->expects($this->any())->method('setRedirect')->willReturnSelf();
148+
$this->userMock->expects($this->any())->method('load')->with($userId)->willReturn($this->userFactoryMock);
146149
$this->userMock->expects($this->any())->method('delete')->willReturnSelf();
147150
$this->messageManagerMock->expects($this->once())->method($resultMethod);
148151

@@ -172,10 +175,12 @@ public function testEmptyPassword()
172175

173176
$this->requestMock->expects($this->any())
174177
->method('getPost')
175-
->willReturnMap([
176-
['user_id', $userId],
177-
[\Magento\User\Block\User\Edit\Tab\Main::CURRENT_USER_PASSWORD_FIELD, ''],
178-
]);
178+
->willReturnMap(
179+
[
180+
['user_id', $userId],
181+
[\Magento\User\Block\User\Edit\Tab\Main::CURRENT_USER_PASSWORD_FIELD, ''],
182+
]
183+
);
179184

180185
$result = $this->controller->execute();
181186
$this->assertNull($result);
@@ -191,8 +196,8 @@ public function executeDataProvider()
191196
return [
192197
[
193198
'currentUserPassword' => '123123q',
194-
'userId' => 1,
195-
'currentUserId' => 2,
199+
'userId' => 2,
200+
'currentUserId' => 1,
196201
'resultMethod' => 'addSuccess',
197202
],
198203
[

‎dev/tests/integration/testsuite/Magento/User/Controller/Adminhtml/User/DeleteTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ public function testDeleteActionWithError()
2222
$messageManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()
2323
->get(\Magento\Framework\Message\ManagerInterface::class);
2424
$user->load(1);
25+
$this->getRequest()->setMethod('POST');
2526
$this->getRequest()->setPostValue('user_id', $user->getId() . '_suffix_ignored_in_mysql_casting_to_int');
2627

2728
$this->dispatch('backend/admin/user/delete');

‎lib/internal/Magento/Framework/Data/Form/Element/Image.php

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,15 @@
44
* See COPYING.txt for license details.
55
*/
66

7+
namespace Magento\Framework\Data\Form\Element;
8+
9+
use Magento\Framework\UrlInterface;
10+
711
/**
812
* Category form input image element
913
*
1014
* @author Magento Core Team <core@magentocommerce.com>
1115
*/
12-
namespace Magento\Framework\Data\Form\Element;
13-
14-
use Magento\Framework\UrlInterface;
15-
1616
class Image extends \Magento\Framework\Data\Form\Element\AbstractElement
1717
{
1818
/**
@@ -54,6 +54,7 @@ public function getElementHtml()
5454
if (!preg_match("/^http\:\/\/|https\:\/\//", $url)) {
5555
$url = $this->_urlBuilder->getBaseUrl(['_type' => UrlInterface::URL_TYPE_MEDIA]) . $url;
5656
}
57+
$url = $this->_escaper->escapeUrl($url);
5758

5859
$html = '<a href="' .
5960
$url .
@@ -70,10 +71,10 @@ public function getElementHtml()
7071
'" id="' .
7172
$this->getHtmlId() .
7273
'_image" title="' .
73-
$this->getValue() .
74+
$this->_escaper->escapeHtmlAttr($this->getValue()) .
7475
'"' .
7576
' alt="' .
76-
$this->getValue() .
77+
$this->_escaper->escapeHtmlAttr($this->getValue()) .
7778
'" height="22" width="22" class="small-image-preview v-middle" ' .
7879
$this->_getUiId() .
7980
' />' .
@@ -127,7 +128,8 @@ protected function _getDeleteCheckbox()
127128
*/
128129
protected function _getHiddenInput()
129130
{
130-
return '<input type="hidden" name="' . parent::getName() . '[value]" value="' . $this->getValue() . '" />';
131+
return '<input type="hidden" name="' . parent::getName() . '[value]" value="' .
132+
$this->_escaper->escapeHtmlAttr($this->getValue()) . '" />';
131133
}
132134

133135
/**

‎lib/internal/Magento/Framework/Data/Test/Unit/Form/Element/ImageTest.php

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
*/
66

77
/**
8-
* Tests for \Magento\Framework\Data\Form\Element\Image
8+
* Tests for \Magento\Framework\Data\Form\Element\Image.
99
*/
1010
namespace Magento\Framework\Data\Test\Unit\Form\Element;
1111

@@ -28,16 +28,24 @@ class ImageTest extends \PHPUnit\Framework\TestCase
2828
*/
2929
protected $_image;
3030

31+
/**
32+
* @var \Magento\Framework\Escaper|\PHPUnit\Framework\MockObject\MockObject
33+
*/
34+
private $escaperMock;
35+
36+
/**
37+
* @inheritdoc
38+
*/
3139
protected function setUp()
3240
{
3341
$factoryMock = $this->createMock(\Magento\Framework\Data\Form\Element\Factory::class);
3442
$collectionFactoryMock = $this->createMock(\Magento\Framework\Data\Form\Element\CollectionFactory::class);
35-
$escaperMock = $this->createMock(\Magento\Framework\Escaper::class);
43+
$this->escaperMock = $this->createMock(\Magento\Framework\Escaper::class);
3644
$this->urlBuilder = $this->createMock(\Magento\Framework\Url::class);
3745
$this->_image = new \Magento\Framework\Data\Form\Element\Image(
3846
$factoryMock,
3947
$collectionFactoryMock,
40-
$escaperMock,
48+
$this->escaperMock,
4149
$this->urlBuilder
4250
);
4351
$formMock = new \Magento\Framework\DataObject();
@@ -47,6 +55,8 @@ protected function setUp()
4755
}
4856

4957
/**
58+
* Check that getType return correct value.
59+
*
5060
* @covers \Magento\Framework\Data\Form\Element\Image::__construct
5161
*/
5262
public function testConstruct()
@@ -55,20 +65,26 @@ public function testConstruct()
5565
}
5666

5767
/**
68+
* Get name and check data.
69+
*
5870
* @covers \Magento\Framework\Data\Form\Element\Image::getName
5971
*/
6072
public function testGetName()
6173
{
6274
$this->_image->setName('image_name');
75+
6376
$this->assertEquals('image_name', $this->_image->getName());
6477
}
6578

6679
/**
80+
* Get element without value and check data.
81+
*
6782
* @covers \Magento\Framework\Data\Form\Element\Image::getElementHtml
6883
*/
6984
public function testGetElementHtmlWithoutValue()
7085
{
7186
$html = $this->_image->getElementHtml();
87+
7288
$this->assertContains('class="input-file"', $html);
7389
$this->assertContains('<input', $html);
7490
$this->assertContains('type="file"', $html);
@@ -77,16 +93,26 @@ public function testGetElementHtmlWithoutValue()
7793
}
7894

7995
/**
96+
* Get element with value and check data.
97+
*
8098
* @covers \Magento\Framework\Data\Form\Element\Image::getElementHtml
8199
*/
82100
public function testGetElementHtmlWithValue()
83101
{
84-
$this->_image->setValue('test_value');
102+
$data = 'test_value';
103+
$baseUrl = 'http://localhost/media/';
104+
$this->_image->setValue($data);
85105
$this->urlBuilder->expects($this->once())
86106
->method('getBaseUrl')
87107
->with(['_type' => UrlInterface::URL_TYPE_MEDIA])
88-
->willReturn('http://localhost/media/');
108+
->willReturn($baseUrl);
109+
$this->escaperMock->expects($this->once())
110+
->method('escapeUrl')
111+
->with($baseUrl . $data)
112+
->willReturn($baseUrl . $data);
113+
$this->escaperMock->expects($this->exactly(3))->method('escapeHtmlAttr')->with($data)->willReturn($data);
89114
$html = $this->_image->getElementHtml();
115+
90116
$this->assertContains('class="input-file"', $html);
91117
$this->assertContains('<input', $html);
92118
$this->assertContains('type="file"', $html);

0 commit comments

Comments
 (0)