Skip to content

Commit 448cbc7

Browse files
VladimirZaetsVladimirZaets
authored
ENGCOM-6349: add check for attribute option values #25421 
2 parents 49606f1 + 47fda42 commit 448cbc7

File tree

3 files changed

+170
-27
lines changed

3 files changed

+170
-27
lines changed

‎app/code/Magento/Catalog/Controller/Adminhtml/Product/Attribute/Validate.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ private function checkUniqueOption(DataObject $response, array $options = null)
252252
private function checkEmptyOption(DataObject $response, array $optionsForCheck = null)
253253
{
254254
foreach ($optionsForCheck as $optionValues) {
255-
if (isset($optionValues[0]) && $optionValues[0] == '') {
255+
if (isset($optionValues[0]) && trim($optionValues[0]) == '') {
256256
$this->setMessageToResponse($response, [__("The value of Admin scope can't be empty.")]);
257257
$response->setError(true);
258258
}

‎app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Product/Attribute/ValidateTest.php

Lines changed: 167 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -136,18 +136,22 @@ public function testExecute()
136136
$serializedOptions = '{"key":"value"}';
137137
$this->requestMock->expects($this->any())
138138
->method('getParam')
139-
->willReturnMap([
139+
->willReturnMap(
140+
[
140141
['frontend_label', null, 'test_frontend_label'],
141142
['attribute_code', null, 'test_attribute_code'],
142143
['new_attribute_set_name', null, 'test_attribute_set_name'],
143144
['serialized_options', '[]', $serializedOptions],
144-
]);
145+
]
146+
);
145147
$this->objectManagerMock->expects($this->exactly(2))
146148
->method('create')
147-
->willReturnMap([
149+
->willReturnMap(
150+
[
148151
[\Magento\Catalog\Model\ResourceModel\Eav\Attribute::class, [], $this->attributeMock],
149152
[\Magento\Eav\Model\Entity\Attribute\Set::class, [], $this->attributeSetMock]
150-
]);
153+
]
154+
);
151155
$this->attributeMock->expects($this->once())
152156
->method('loadByCode')
153157
->willReturnSelf();
@@ -182,23 +186,25 @@ public function testExecute()
182186

183187
/**
184188
* @dataProvider provideUniqueData
185-
* @param array $options
186-
* @param boolean $isError
187-
* @throws \Magento\Framework\Exception\NotFoundException
189+
* @param array $options
190+
* @param boolean $isError
191+
* @throws \Magento\Framework\Exception\NotFoundException
188192
*/
189193
public function testUniqueValidation(array $options, $isError)
190194
{
191195
$serializedOptions = '{"key":"value"}';
192196
$countFunctionCalls = ($isError) ? 6 : 5;
193197
$this->requestMock->expects($this->exactly($countFunctionCalls))
194198
->method('getParam')
195-
->willReturnMap([
199+
->willReturnMap(
200+
[
196201
['frontend_label', null, null],
197202
['attribute_code', null, "test_attribute_code"],
198203
['new_attribute_set_name', null, 'test_attribute_set_name'],
199204
['message_key', null, Validate::DEFAULT_MESSAGE_KEY],
200205
['serialized_options', '[]', $serializedOptions],
201-
]);
206+
]
207+
);
202208

203209
$this->formDataSerializerMock
204210
->expects($this->once())
@@ -323,22 +329,24 @@ public function provideUniqueData()
323329
* Check that empty admin scope labels will trigger error.
324330
*
325331
* @dataProvider provideEmptyOption
326-
* @param array $options
327-
* @throws \Magento\Framework\Exception\NotFoundException
332+
* @param array $options
333+
* @throws \Magento\Framework\Exception\NotFoundException
328334
*/
329335
public function testEmptyOption(array $options, $result)
330336
{
331337
$serializedOptions = '{"key":"value"}';
332338
$this->requestMock->expects($this->any())
333339
->method('getParam')
334-
->willReturnMap([
340+
->willReturnMap(
341+
[
335342
['frontend_label', null, null],
336343
['frontend_input', 'select', 'multipleselect'],
337344
['attribute_code', null, "test_attribute_code"],
338345
['new_attribute_set_name', null, 'test_attribute_set_name'],
339346
['message_key', Validate::DEFAULT_MESSAGE_KEY, 'message'],
340347
['serialized_options', '[]', $serializedOptions],
341-
]);
348+
]
349+
);
342350

343351
$this->formDataSerializerMock
344352
->expects($this->once())
@@ -439,6 +447,129 @@ public function provideEmptyOption()
439447
];
440448
}
441449

450+
/**
451+
* Check that admin scope labels which only contain spaces will trigger error.
452+
*
453+
* @dataProvider provideWhitespaceOption
454+
* @param array $options
455+
* @param $result
456+
* @throws \Magento\Framework\Exception\NotFoundException
457+
*/
458+
public function testWhitespaceOption(array $options, $result)
459+
{
460+
$serializedOptions = '{"key":"value"}';
461+
$this->requestMock->expects($this->any())
462+
->method('getParam')
463+
->willReturnMap(
464+
[
465+
['frontend_label', null, null],
466+
['frontend_input', 'select', 'multipleselect'],
467+
['attribute_code', null, "test_attribute_code"],
468+
['new_attribute_set_name', null, 'test_attribute_set_name'],
469+
['message_key', Validate::DEFAULT_MESSAGE_KEY, 'message'],
470+
['serialized_options', '[]', $serializedOptions],
471+
]
472+
);
473+
474+
$this->formDataSerializerMock
475+
->expects($this->once())
476+
->method('unserialize')
477+
->with($serializedOptions)
478+
->willReturn($options);
479+
480+
$this->objectManagerMock->expects($this->once())
481+
->method('create')
482+
->willReturn($this->attributeMock);
483+
484+
$this->attributeMock->expects($this->once())
485+
->method('loadByCode')
486+
->willReturnSelf();
487+
488+
$this->attributeCodeValidatorMock->expects($this->once())
489+
->method('isValid')
490+
->with('test_attribute_code')
491+
->willReturn(true);
492+
493+
$this->resultJsonFactoryMock->expects($this->once())
494+
->method('create')
495+
->willReturn($this->resultJson);
496+
497+
$this->resultJson->expects($this->once())
498+
->method('setJsonData')
499+
->willReturnArgument(0);
500+
501+
$response = $this->getModel()->execute();
502+
$responseObject = json_decode($response);
503+
$this->assertEquals($responseObject, $result);
504+
}
505+
506+
/**
507+
* Dataprovider for testWhitespaceOption.
508+
*
509+
* @return array
510+
*/
511+
public function provideWhitespaceOption()
512+
{
513+
return [
514+
'whitespace admin scope options' => [
515+
[
516+
'option' => [
517+
'value' => [
518+
"option_0" => [' '],
519+
],
520+
],
521+
],
522+
(object) [
523+
'error' => true,
524+
'message' => 'The value of Admin scope can\'t be empty.',
525+
]
526+
],
527+
'not empty admin scope options' => [
528+
[
529+
'option' => [
530+
'value' => [
531+
"option_0" => ['asdads'],
532+
],
533+
],
534+
],
535+
(object) [
536+
'error' => false,
537+
]
538+
],
539+
'whitespace admin scope options and deleted' => [
540+
[
541+
'option' => [
542+
'value' => [
543+
"option_0" => [' '],
544+
],
545+
'delete' => [
546+
'option_0' => '1',
547+
],
548+
],
549+
],
550+
(object) [
551+
'error' => false,
552+
],
553+
],
554+
'whitespace admin scope options and not deleted' => [
555+
[
556+
'option' => [
557+
'value' => [
558+
"option_0" => [' '],
559+
],
560+
'delete' => [
561+
'option_0' => '0',
562+
],
563+
],
564+
],
565+
(object) [
566+
'error' => true,
567+
'message' => 'The value of Admin scope can\'t be empty.',
568+
],
569+
],
570+
];
571+
}
572+
442573
/**
443574
* @throws \Magento\Framework\Exception\NotFoundException
444575
*/
@@ -449,13 +580,15 @@ public function testExecuteWithOptionsDataError()
449580
. "If the error persists, please try again later.";
450581
$this->requestMock->expects($this->any())
451582
->method('getParam')
452-
->willReturnMap([
583+
->willReturnMap(
584+
[
453585
['frontend_label', null, 'test_frontend_label'],
454586
['attribute_code', null, 'test_attribute_code'],
455587
['new_attribute_set_name', null, 'test_attribute_set_name'],
456588
['message_key', Validate::DEFAULT_MESSAGE_KEY, 'message'],
457589
['serialized_options', '[]', $serializedOptions],
458-
]);
590+
]
591+
);
459592

460593
$this->formDataSerializerMock
461594
->expects($this->once())
@@ -465,10 +598,12 @@ public function testExecuteWithOptionsDataError()
465598

466599
$this->objectManagerMock
467600
->method('create')
468-
->willReturnMap([
601+
->willReturnMap(
602+
[
469603
[\Magento\Catalog\Model\ResourceModel\Eav\Attribute::class, [], $this->attributeMock],
470604
[\Magento\Eav\Model\Entity\Attribute\Set::class, [], $this->attributeSetMock]
471-
]);
605+
]
606+
);
472607

473608
$this->attributeCodeValidatorMock
474609
->method('isValid')
@@ -485,10 +620,14 @@ public function testExecuteWithOptionsDataError()
485620
->willReturn($this->resultJson);
486621
$this->resultJson->expects($this->once())
487622
->method('setJsonData')
488-
->with(json_encode([
489-
'error' => true,
490-
'message' => $message
491-
]))
623+
->with(
624+
json_encode(
625+
[
626+
'error' => true,
627+
'message' => $message
628+
]
629+
)
630+
)
492631
->willReturnSelf();
493632

494633
$this->getModel()->execute();
@@ -498,23 +637,25 @@ public function testExecuteWithOptionsDataError()
498637
* Test execute with an invalid attribute code
499638
*
500639
* @dataProvider provideInvalidAttributeCodes
501-
* @param string $attributeCode
502-
* @param $result
503-
* @throws \Magento\Framework\Exception\NotFoundException
640+
* @param string $attributeCode
641+
* @param $result
642+
* @throws \Magento\Framework\Exception\NotFoundException
504643
*/
505644
public function testExecuteWithInvalidAttributeCode($attributeCode, $result)
506645
{
507646
$serializedOptions = '{"key":"value"}';
508647
$this->requestMock->expects($this->any())
509648
->method('getParam')
510-
->willReturnMap([
649+
->willReturnMap(
650+
[
511651
['frontend_label', null, null],
512652
['frontend_input', 'select', 'multipleselect'],
513653
['attribute_code', null, $attributeCode],
514654
['new_attribute_set_name', null, 'test_attribute_set_name'],
515655
['message_key', Validate::DEFAULT_MESSAGE_KEY, 'message'],
516656
['serialized_options', '[]', $serializedOptions],
517-
]);
657+
]
658+
);
518659

519660
$this->formDataSerializerMock
520661
->expects($this->once())

‎app/code/Magento/ConfigurableProduct/view/adminhtml/web/js/variations/steps/attributes_values.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,8 @@ define([
106106
errorOption,
107107
allOptions = [];
108108

109+
newOption.label = $.trim(newOption.label);
110+
109111
if (_.isEmpty(newOption.label)) {
110112
return false;
111113
}

0 commit comments

Comments
 (0)