diff --git a/app/code/Magento/Backend/Test/Mftf/Section/AdminMessagesSection.xml b/app/code/Magento/Backend/Test/Mftf/Section/AdminMessagesSection.xml index 72a00ed6db9b6..4af66986d9aa8 100644 --- a/app/code/Magento/Backend/Test/Mftf/Section/AdminMessagesSection.xml +++ b/app/code/Magento/Backend/Test/Mftf/Section/AdminMessagesSection.xml @@ -11,5 +11,6 @@ + diff --git a/app/code/Magento/Catalog/Controller/Adminhtml/Product/Attribute/Save.php b/app/code/Magento/Catalog/Controller/Adminhtml/Product/Attribute/Save.php index b9f9b739f4fa3..fcd0fbc03278a 100644 --- a/app/code/Magento/Catalog/Controller/Adminhtml/Product/Attribute/Save.php +++ b/app/code/Magento/Catalog/Controller/Adminhtml/Product/Attribute/Save.php @@ -18,6 +18,8 @@ use Magento\Eav\Model\Adminhtml\System\Config\Source\Inputtype\Validator; use Magento\Eav\Model\Adminhtml\System\Config\Source\Inputtype\ValidatorFactory; use Magento\Eav\Model\ResourceModel\Entity\Attribute\Group\CollectionFactory; +use Magento\Framework\App\ObjectManager; +use Magento\Framework\Serialize\Serializer\FormData; use Magento\Framework\Cache\FrontendInterface; use Magento\Framework\Controller\ResultFactory; use Magento\Framework\Controller\Result\Json; @@ -68,6 +70,11 @@ class Save extends Attribute */ private $layoutFactory; + /** + * @var FormData + */ + private $formDataSerializer; + /** * @param Context $context * @param FrontendInterface $attributeLabelCache @@ -80,6 +87,7 @@ class Save extends Attribute * @param FilterManager $filterManager * @param Product $productHelper * @param LayoutFactory $layoutFactory + * @param FormData|null $formDataSerializer * @SuppressWarnings(PHPMD.ExcessiveParameterList) */ public function __construct( @@ -93,7 +101,8 @@ public function __construct( CollectionFactory $groupCollectionFactory, FilterManager $filterManager, Product $productHelper, - LayoutFactory $layoutFactory + LayoutFactory $layoutFactory, + FormData $formDataSerializer = null ) { parent::__construct($context, $attributeLabelCache, $coreRegistry, $resultPageFactory); $this->buildFactory = $buildFactory; @@ -103,19 +112,37 @@ public function __construct( $this->validatorFactory = $validatorFactory; $this->groupCollectionFactory = $groupCollectionFactory; $this->layoutFactory = $layoutFactory; + $this->formDataSerializer = $formDataSerializer ?? ObjectManager::getInstance()->get(FormData::class); } /** - * @return Redirect + * @inheritdoc + * * @SuppressWarnings(PHPMD.CyclomaticComplexity) * @SuppressWarnings(PHPMD.NPathComplexity) * @SuppressWarnings(PHPMD.ExcessiveMethodLength) */ public function execute() { + try { + $optionData = $this->formDataSerializer->unserialize( + $this->getRequest()->getParam('serialized_options', '[]') + ); + } catch (\InvalidArgumentException $e) { + $message = __("The attribute couldn't be saved due to an error. Verify your information and try again. " + . "If the error persists, please try again later."); + $this->messageManager->addErrorMessage($message); + + return $this->returnResult('catalog/*/edit', ['_current' => true], ['error' => true]); + } + $data = $this->getRequest()->getPostValue(); + $data = array_replace_recursive( + $data, + $optionData + ); + if ($data) { - $this->preprocessOptionsData($data); $setId = $this->getRequest()->getParam('set'); $attributeSet = null; @@ -124,7 +151,7 @@ public function execute() $name = trim($name); try { - /** @var $attributeSet Set */ + /** @var Set $attributeSet */ $attributeSet = $this->buildFactory->create() ->setEntityTypeId($this->_entityTypeId) ->setSkeletonId($setId) @@ -147,7 +174,7 @@ public function execute() $attributeId = $this->getRequest()->getParam('attribute_id'); - /** @var $model ProductAttributeInterface */ + /** @var ProductAttributeInterface $model */ $model = $this->attributeFactory->create(); if ($attributeId) { $model->load($attributeId); @@ -180,7 +207,7 @@ public function execute() //validate frontend_input if (isset($data['frontend_input'])) { - /** @var $inputType Validator */ + /** @var Validator $inputType */ $inputType = $this->validatorFactory->create(); if (!$inputType->isValid($data['frontend_input'])) { foreach ($inputType->getMessages() as $message) { @@ -313,28 +340,8 @@ public function execute() } /** - * Extract options data from serialized options field and append to data array. - * - * This logic is required to overcome max_input_vars php limit - * that may vary and/or be inaccessible to change on different instances. + * Provides an initialized Result object. * - * @param array $data - * @return void - */ - private function preprocessOptionsData(&$data) - { - if (isset($data['serialized_options'])) { - $serializedOptions = json_decode($data['serialized_options'], JSON_OBJECT_AS_ARRAY); - foreach ($serializedOptions as $serializedOption) { - $option = []; - parse_str($serializedOption, $option); - $data = array_replace_recursive($data, $option); - } - } - unset($data['serialized_options']); - } - - /** * @param string $path * @param array $params * @param array $response diff --git a/app/code/Magento/Catalog/Controller/Adminhtml/Product/Attribute/Validate.php b/app/code/Magento/Catalog/Controller/Adminhtml/Product/Attribute/Validate.php index 7fe012a87d929..e56428a1ae77e 100644 --- a/app/code/Magento/Catalog/Controller/Adminhtml/Product/Attribute/Validate.php +++ b/app/code/Magento/Catalog/Controller/Adminhtml/Product/Attribute/Validate.php @@ -6,8 +6,15 @@ */ namespace Magento\Catalog\Controller\Adminhtml\Product\Attribute; +use Magento\Framework\Serialize\Serializer\FormData; +use Magento\Framework\App\ObjectManager; use Magento\Framework\DataObject; +/** + * Product attribute validate controller. + * + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) + */ class Validate extends \Magento\Catalog\Controller\Adminhtml\Product\Attribute { const DEFAULT_MESSAGE_KEY = 'message'; @@ -27,6 +34,11 @@ class Validate extends \Magento\Catalog\Controller\Adminhtml\Product\Attribute */ private $multipleAttributeList; + /** + * @var FormData + */ + private $formDataSerializer; + /** * Constructor * @@ -37,6 +49,7 @@ class Validate extends \Magento\Catalog\Controller\Adminhtml\Product\Attribute * @param \Magento\Framework\Controller\Result\JsonFactory $resultJsonFactory * @param \Magento\Framework\View\LayoutFactory $layoutFactory * @param array $multipleAttributeList + * @param FormData|null $formDataSerializer */ public function __construct( \Magento\Backend\App\Action\Context $context, @@ -45,16 +58,19 @@ public function __construct( \Magento\Framework\View\Result\PageFactory $resultPageFactory, \Magento\Framework\Controller\Result\JsonFactory $resultJsonFactory, \Magento\Framework\View\LayoutFactory $layoutFactory, - array $multipleAttributeList = [] + array $multipleAttributeList = [], + FormData $formDataSerializer = null ) { parent::__construct($context, $attributeLabelCache, $coreRegistry, $resultPageFactory); $this->resultJsonFactory = $resultJsonFactory; $this->layoutFactory = $layoutFactory; $this->multipleAttributeList = $multipleAttributeList; + $this->formDataSerializer = $formDataSerializer ?? ObjectManager::getInstance()->get(FormData::class); } /** - * @return \Magento\Framework\Controller\ResultInterface + * @inheritdoc + * * @SuppressWarnings(PHPMD.NPathComplexity) * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ @@ -62,6 +78,16 @@ public function execute() { $response = new DataObject(); $response->setError(false); + try { + $optionsData = $this->formDataSerializer->unserialize( + $this->getRequest()->getParam('serialized_options', '[]') + ); + } catch (\InvalidArgumentException $e) { + $message = __("The attribute couldn't be validated due to an error. Verify your information and try again. " + . "If the error persists, please try again later."); + $this->setMessageToResponse($response, [$message]); + $response->setError(true); + } $attributeCode = $this->getRequest()->getParam('attribute_code'); $frontendLabel = $this->getRequest()->getParam('frontend_label'); @@ -101,10 +127,10 @@ public function execute() } $multipleOption = $this->getRequest()->getParam("frontend_input"); - $multipleOption = null == $multipleOption ? 'select' : $multipleOption; + $multipleOption = (null === $multipleOption) ? 'select' : $multipleOption; if (isset($this->multipleAttributeList[$multipleOption]) && !(null == ($multipleOption))) { - $options = $this->getRequest()->getParam($this->multipleAttributeList[$multipleOption]); + $options = $optionsData[$this->multipleAttributeList[$multipleOption]] ?? null; $this->checkUniqueOption( $response, $options @@ -122,7 +148,8 @@ public function execute() } /** - * Throws Exception if not unique values into options + * Throws Exception if not unique values into options. + * * @param array $optionsValues * @param array $deletedOptions * @return bool @@ -156,6 +183,8 @@ private function setMessageToResponse($response, $messages) } /** + * Performs checking the uniqueness of the attribute options. + * * @param DataObject $response * @param array|null $options * @return $this diff --git a/app/code/Magento/Catalog/Test/Mftf/ActionGroup/AdminProductAttributeActionGroup.xml b/app/code/Magento/Catalog/Test/Mftf/ActionGroup/AdminProductAttributeActionGroup.xml new file mode 100644 index 0000000000000..c9841dc0527c8 --- /dev/null +++ b/app/code/Magento/Catalog/Test/Mftf/ActionGroup/AdminProductAttributeActionGroup.xml @@ -0,0 +1,29 @@ + + + + + + + + + + + + + + + + + + + + + + diff --git a/app/code/Magento/Catalog/Test/Mftf/Page/AdminProductAttributeFormPage.xml b/app/code/Magento/Catalog/Test/Mftf/Page/AdminProductAttributeFormPage.xml index 8213ec6ab5fbe..e199e49bb082a 100644 --- a/app/code/Magento/Catalog/Test/Mftf/Page/AdminProductAttributeFormPage.xml +++ b/app/code/Magento/Catalog/Test/Mftf/Page/AdminProductAttributeFormPage.xml @@ -10,5 +10,6 @@
+
diff --git a/app/code/Magento/Catalog/Test/Mftf/Section/AdminCreateProductAttributeSection.xml b/app/code/Magento/Catalog/Test/Mftf/Section/AdminCreateProductAttributeSection.xml index e5e37b59af359..9801fdd58ae53 100644 --- a/app/code/Magento/Catalog/Test/Mftf/Section/AdminCreateProductAttributeSection.xml +++ b/app/code/Magento/Catalog/Test/Mftf/Section/AdminCreateProductAttributeSection.xml @@ -11,5 +11,22 @@
+ + + +
+
+ + + + +
+
+ + + + +
diff --git a/app/code/Magento/Catalog/Test/Mftf/Section/AdminProductAttributeGridSection.xml b/app/code/Magento/Catalog/Test/Mftf/Section/AdminProductAttributeGridSection.xml index bb944270a10a9..386f679f35ccc 100644 --- a/app/code/Magento/Catalog/Test/Mftf/Section/AdminProductAttributeGridSection.xml +++ b/app/code/Magento/Catalog/Test/Mftf/Section/AdminProductAttributeGridSection.xml @@ -12,6 +12,7 @@ + diff --git a/app/code/Magento/Catalog/Test/Mftf/Test/AdminProductStatusAttributeDisabledByDefaultTest.xml b/app/code/Magento/Catalog/Test/Mftf/Test/AdminProductStatusAttributeDisabledByDefaultTest.xml index 9ec47e5a36464..2ae817c732434 100644 --- a/app/code/Magento/Catalog/Test/Mftf/Test/AdminProductStatusAttributeDisabledByDefaultTest.xml +++ b/app/code/Magento/Catalog/Test/Mftf/Test/AdminProductStatusAttributeDisabledByDefaultTest.xml @@ -28,7 +28,7 @@ - + @@ -42,7 +42,7 @@ - + diff --git a/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Product/Attribute/SaveTest.php b/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Product/Attribute/SaveTest.php index f493cbc88f18e..a1aaab0995d73 100644 --- a/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Product/Attribute/SaveTest.php +++ b/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Product/Attribute/SaveTest.php @@ -3,8 +3,10 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ + namespace Magento\Catalog\Test\Unit\Controller\Adminhtml\Product\Attribute; +use Magento\Catalog\Api\Data\ProductAttributeInterface; use Magento\Catalog\Controller\Adminhtml\Product\Attribute\Save; use Magento\Catalog\Test\Unit\Controller\Adminhtml\Product\AttributeTest; use Magento\Catalog\Model\Product\AttributeSet\BuildFactory; @@ -13,11 +15,16 @@ use Magento\Eav\Api\Data\AttributeSetInterface; use Magento\Eav\Model\Adminhtml\System\Config\Source\Inputtype\ValidatorFactory; use Magento\Eav\Model\ResourceModel\Entity\Attribute\Group\CollectionFactory; +use Magento\Framework\Message\ManagerInterface; +use Magento\Framework\Serialize\Serializer\FormData; +use Magento\Framework\Controller\ResultFactory; use Magento\Framework\Filter\FilterManager; use Magento\Catalog\Helper\Product as ProductHelper; +use Magento\Framework\View\Element\Messages; use Magento\Framework\View\LayoutFactory; use Magento\Backend\Model\View\Result\Redirect as ResultRedirect; use Magento\Eav\Model\Adminhtml\System\Config\Source\Inputtype\Validator as InputTypeValidator; +use Magento\Framework\View\LayoutInterface; /** * @SuppressWarnings(PHPMD.CouplingBetweenObjects) @@ -79,6 +86,21 @@ class SaveTest extends AttributeTest */ protected $inputTypeValidatorMock; + /** + * @var ManagerInterface|\PHPUnit_Framework_MockObject_MockObject + */ + private $messageManagerMock; + + /** + * @var FormData|\PHPUnit_Framework_MockObject_MockObject + */ + private $formDataSerializerMock; + + /** + * @var ProductAttributeInterface|\PHPUnit_Framework_MockObject_MockObject + */ + private $productAttributeMock; + protected function setUp() { parent::setUp(); @@ -108,6 +130,7 @@ protected function setUp() ->disableOriginalConstructor() ->getMock(); $this->redirectMock = $this->getMockBuilder(ResultRedirect::class) + ->setMethods(['setData', 'setPath']) ->disableOriginalConstructor() ->getMock(); $this->attributeSetMock = $this->getMockBuilder(AttributeSetInterface::class) @@ -119,6 +142,15 @@ protected function setUp() $this->inputTypeValidatorMock = $this->getMockBuilder(InputTypeValidator::class) ->disableOriginalConstructor() ->getMock(); + $this->messageManagerMock = $this->getMockBuilder(\Magento\Framework\Message\ManagerInterface::class) + ->disableOriginalConstructor() + ->getMockForAbstractClass(); + $this->formDataSerializerMock = $this->getMockBuilder(FormData::class) + ->disableOriginalConstructor() + ->getMock(); + $this->productAttributeMock = $this->getMockBuilder(ProductAttributeInterface::class) + ->setMethods(['getId', 'get']) + ->getMockForAbstractClass(); $this->buildFactoryMock->expects($this->any()) ->method('create') @@ -126,6 +158,9 @@ protected function setUp() $this->validatorFactoryMock->expects($this->any()) ->method('create') ->willReturn($this->inputTypeValidatorMock); + $this->attributeFactoryMock + ->method('create') + ->willReturn($this->productAttributeMock); } /** @@ -135,6 +170,7 @@ protected function getModel() { return $this->objectManager->getObject(Save::class, [ 'context' => $this->contextMock, + 'messageManager' => $this->messageManagerMock, 'attributeLabelCache' => $this->attributeLabelCacheMock, 'coreRegistry' => $this->coreRegistryMock, 'resultPageFactory' => $this->resultPageFactoryMock, @@ -145,11 +181,22 @@ protected function getModel() 'validatorFactory' => $this->validatorFactoryMock, 'groupCollectionFactory' => $this->groupCollectionFactoryMock, 'layoutFactory' => $this->layoutFactoryMock, + 'formDataSerializer' => $this->formDataSerializerMock, ]); } public function testExecuteWithEmptyData() { + $this->requestMock->expects($this->any()) + ->method('getParam') + ->willReturnMap([ + ['isAjax', null, null], + ['serialized_options', '[]', ''], + ]); + $this->formDataSerializerMock->expects($this->once()) + ->method('unserialize') + ->with('') + ->willReturn([]); $this->requestMock->expects($this->once()) ->method('getPostValue') ->willReturn([]); @@ -170,6 +217,22 @@ public function testExecute() 'frontend_input' => 'test_frontend_input', ]; + $this->requestMock->expects($this->any()) + ->method('getParam') + ->willReturnMap([ + ['isAjax', null, null], + ['serialized_options', '[]', ''], + ]); + $this->formDataSerializerMock->expects($this->once()) + ->method('unserialize') + ->with('') + ->willReturn([]); + $this->productAttributeMock->expects($this->once()) + ->method('getId') + ->willReturn(1); + $this->productAttributeMock->expects($this->once()) + ->method('getAttributeCode') + ->willReturn('test_code'); $this->requestMock->expects($this->once()) ->method('getPostValue') ->willReturn($data); @@ -203,4 +266,74 @@ public function testExecute() $this->assertInstanceOf(ResultRedirect::class, $this->getModel()->execute()); } + + /** + * @return void + * @throws \Magento\Framework\Exception\NotFoundException + */ + public function testExecuteWithOptionsDataError() + { + $serializedOptions = '{"key":"value"}'; + $message = "The attribute couldn't be saved due to an error. Verify your information and try again. " + . "If the error persists, please try again later."; + + $this->requestMock->expects($this->any()) + ->method('getParam') + ->willReturnMap([ + ['isAjax', null, true], + ['serialized_options', '[]', $serializedOptions], + ]); + $this->formDataSerializerMock->expects($this->once()) + ->method('unserialize') + ->with($serializedOptions) + ->willThrowException(new \InvalidArgumentException('Some exception')); + $this->messageManagerMock->expects($this->once()) + ->method('addErrorMessage') + ->with($message); + $this->addReturnResultConditions('catalog/*/edit', ['_current' => true], ['error' => true]); + + $this->getModel()->execute(); + } + + /** + * @param string $path + * @param array $params + * @param array $response + * @return void + * @SuppressWarnings(PHPMD.UnusedFormalParameter) + */ + private function addReturnResultConditions(string $path = '', array $params = [], array $response = []) + { + $layoutMock = $this->getMockBuilder(LayoutInterface::class) + ->setMethods(['initMessages', 'getMessagesBlock']) + ->getMockForAbstractClass(); + $this->layoutFactoryMock->expects($this->once()) + ->method('create') + ->with() + ->willReturn($layoutMock); + $layoutMock->expects($this->once()) + ->method('initMessages') + ->with(); + $messageBlockMock = $this->getMockBuilder(Messages::class) + ->disableOriginalConstructor() + ->getMock(); + $layoutMock->expects($this->once()) + ->method('getMessagesBlock') + ->willReturn($messageBlockMock); + $messageBlockMock->expects($this->once()) + ->method('getGroupedHtml') + ->willReturn('message1'); + $this->resultFactoryMock->expects($this->once()) + ->method('create') + ->with(ResultFactory::TYPE_JSON) + ->willReturn($this->redirectMock); + $response = array_merge($response, [ + 'messages' => ['message1'], + 'params' => $params, + ]); + $this->redirectMock->expects($this->once()) + ->method('setData') + ->with($response) + ->willReturnSelf(); + } } diff --git a/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Product/Attribute/ValidateTest.php b/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Product/Attribute/ValidateTest.php index 9c747393cc72a..750d38f60e13d 100644 --- a/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Product/Attribute/ValidateTest.php +++ b/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Product/Attribute/ValidateTest.php @@ -9,6 +9,7 @@ use Magento\Catalog\Model\ResourceModel\Eav\Attribute; use Magento\Catalog\Test\Unit\Controller\Adminhtml\Product\AttributeTest; use Magento\Eav\Model\Entity\Attribute\Set as AttributeSet; +use Magento\Framework\Serialize\Serializer\FormData; use Magento\Framework\Controller\Result\Json as ResultJson; use Magento\Framework\Controller\Result\JsonFactory as ResultJsonFactory; use Magento\Framework\Escaper; @@ -61,6 +62,11 @@ class ValidateTest extends AttributeTest */ protected $layoutMock; + /** + * @var FormData|\PHPUnit_Framework_MockObject_MockObject + */ + private $formDataSerializer; + protected function setUp() { parent::setUp(); @@ -86,6 +92,9 @@ protected function setUp() ->getMock(); $this->layoutMock = $this->getMockBuilder(LayoutInterface::class) ->getMockForAbstractClass(); + $this->formDataSerializer = $this->getMockBuilder(FormData::class) + ->disableOriginalConstructor() + ->getMock(); $this->contextMock->expects($this->any()) ->method('getObjectManager') @@ -100,25 +109,28 @@ protected function getModel() return $this->objectManager->getObject( Validate::class, [ - 'context' => $this->contextMock, - 'attributeLabelCache' => $this->attributeLabelCacheMock, - 'coreRegistry' => $this->coreRegistryMock, - 'resultPageFactory' => $this->resultPageFactoryMock, - 'resultJsonFactory' => $this->resultJsonFactoryMock, - 'layoutFactory' => $this->layoutFactoryMock, - 'multipleAttributeList' => ['select' => 'option'] + 'context' => $this->contextMock, + 'attributeLabelCache' => $this->attributeLabelCacheMock, + 'coreRegistry' => $this->coreRegistryMock, + 'resultPageFactory' => $this->resultPageFactoryMock, + 'resultJsonFactory' => $this->resultJsonFactoryMock, + 'layoutFactory' => $this->layoutFactoryMock, + 'multipleAttributeList' => ['select' => 'option'], + 'formDataSerializer' => $this->formDataSerializer, ] ); } public function testExecute() { + $serializedOptions = '{"key":"value"}'; $this->requestMock->expects($this->any()) ->method('getParam') ->willReturnMap([ ['frontend_label', null, 'test_frontend_label'], ['attribute_code', null, 'test_attribute_code'], ['new_attribute_set_name', null, 'test_attribute_set_name'], + ['serialized_options', '[]', $serializedOptions], ]); $this->objectManagerMock->expects($this->exactly(2)) ->method('create') @@ -160,6 +172,7 @@ public function testExecute() */ public function testUniqueValidation(array $options, $isError) { + $serializedOptions = '{"key":"value"}'; $countFunctionCalls = ($isError) ? 6 : 5; $this->requestMock->expects($this->exactly($countFunctionCalls)) ->method('getParam') @@ -167,10 +180,15 @@ public function testUniqueValidation(array $options, $isError) ['frontend_label', null, null], ['attribute_code', null, "test_attribute_code"], ['new_attribute_set_name', null, 'test_attribute_set_name'], - ['option', null, $options], - ['message_key', null, Validate::DEFAULT_MESSAGE_KEY] + ['message_key', null, Validate::DEFAULT_MESSAGE_KEY], + ['serialized_options', '[]', $serializedOptions], ]); + $this->formDataSerializer->expects($this->once()) + ->method('unserialize') + ->with($serializedOptions) + ->willReturn($options); + $this->objectManagerMock->expects($this->once()) ->method('create') ->willReturn($this->attributeMock); @@ -203,68 +221,84 @@ public function provideUniqueData() return [ 'no values' => [ [ - 'delete' => [ - "option_0" => "", - "option_1" => "", - "option_2" => "", - ] - ], false + 'option' => [ + 'delete' => [ + "option_0" => "", + "option_1" => "", + "option_2" => "", + ], + ], + + ], + false, ], 'valid options' => [ [ - 'value' => [ - "option_0" => [1, 0], - "option_1" => [2, 0], - "option_2" => [3, 0], + 'option' => [ + 'value' => [ + "option_0" => [1, 0], + "option_1" => [2, 0], + "option_2" => [3, 0], + ], + 'delete' => [ + "option_0" => "", + "option_1" => "", + "option_2" => "", + ], ], - 'delete' => [ - "option_0" => "", - "option_1" => "", - "option_2" => "", - ] - ], false + ], + false, ], 'duplicate options' => [ [ - 'value' => [ - "option_0" => [1, 0], - "option_1" => [1, 0], - "option_2" => [3, 0], + 'option' => [ + 'value' => [ + "option_0" => [1, 0], + "option_1" => [1, 0], + "option_2" => [3, 0], + ], + 'delete' => [ + "option_0" => "", + "option_1" => "", + "option_2" => "", + ], ], - 'delete' => [ - "option_0" => "", - "option_1" => "", - "option_2" => "", - ] - ], true + ], + true, ], 'duplicate and deleted' => [ [ - 'value' => [ - "option_0" => [1, 0], - "option_1" => [1, 0], - "option_2" => [3, 0], + 'option' => [ + 'value' => [ + "option_0" => [1, 0], + "option_1" => [1, 0], + "option_2" => [3, 0], + ], + 'delete' => [ + "option_0" => "", + "option_1" => "1", + "option_2" => "", + ], ], - 'delete' => [ - "option_0" => "", - "option_1" => "1", - "option_2" => "", - ] - ], false + ], + false, ], 'empty and deleted' => [ [ - 'value' => [ - "option_0" => [1, 0], - "option_1" => [2, 0], - "option_2" => ["", ""], + 'option' => [ + 'value' => [ + "option_0" => [1, 0], + "option_1" => [2, 0], + "option_2" => ["", ""], + ], + 'delete' => [ + "option_0" => "", + "option_1" => "", + "option_2" => "1", + ], ], - 'delete' => [ - "option_0" => "", - "option_1" => "", - "option_2" => "1", - ] - ], false + ], + false, ], ]; } @@ -278,6 +312,7 @@ public function provideUniqueData() */ public function testEmptyOption(array $options, $result) { + $serializedOptions = '{"key":"value"}'; $this->requestMock->expects($this->any()) ->method('getParam') ->willReturnMap([ @@ -285,10 +320,15 @@ public function testEmptyOption(array $options, $result) ['frontend_input', 'select', 'multipleselect'], ['attribute_code', null, "test_attribute_code"], ['new_attribute_set_name', null, 'test_attribute_set_name'], - ['option', null, $options], ['message_key', Validate::DEFAULT_MESSAGE_KEY, 'message'], + ['serialized_options', '[]', $serializedOptions], ]); + $this->formDataSerializer->expects($this->once()) + ->method('unserialize') + ->with($serializedOptions) + ->willReturn($options); + $this->objectManagerMock->expects($this->once()) ->method('create') ->willReturn($this->attributeMock); @@ -320,32 +360,38 @@ public function provideEmptyOption() return [ 'empty admin scope options' => [ [ - 'value' => [ - "option_0" => [''], + 'option' => [ + 'value' => [ + "option_0" => [''], + ], ], ], (object) [ 'error' => true, 'message' => 'The value of Admin scope can\'t be empty.', - ] + ], ], 'not empty admin scope options' => [ [ - 'value' => [ - "option_0" => ['asdads'], + 'option' => [ + 'value' => [ + "option_0" => ['asdads'], + ], ], ], (object) [ 'error' => false, - ] + ], ], 'empty admin scope options and deleted' => [ [ - 'value' => [ - "option_0" => [''], - ], - 'delete' => [ - 'option_0' => '1', + 'option' => [ + 'value' => [ + "option_0" => [''], + ], + 'delete' => [ + 'option_0' => '1', + ], ], ], (object) [ @@ -354,11 +400,13 @@ public function provideEmptyOption() ], 'empty admin scope options and not deleted' => [ [ - 'value' => [ - "option_0" => [''], - ], - 'delete' => [ - 'option_0' => '0', + 'option' => [ + 'value' => [ + "option_0" => [''], + ], + 'delete' => [ + 'option_0' => '0', + ], ], ], (object) [ @@ -368,4 +416,55 @@ public function provideEmptyOption() ], ]; } + + /** + * @return void + * @throws \Magento\Framework\Exception\NotFoundException + */ + public function testExecuteWithOptionsDataError() + { + $serializedOptions = '{"key":"value"}'; + $message = "The attribute couldn't be validated due to an error. Verify your information and try again. " + . "If the error persists, please try again later."; + $this->requestMock->expects($this->any()) + ->method('getParam') + ->willReturnMap([ + ['frontend_label', null, 'test_frontend_label'], + ['attribute_code', null, 'test_attribute_code'], + ['new_attribute_set_name', null, 'test_attribute_set_name'], + ['message_key', Validate::DEFAULT_MESSAGE_KEY, 'message'], + ['serialized_options', '[]', $serializedOptions], + ]); + + $this->formDataSerializer->expects($this->once()) + ->method('unserialize') + ->with($serializedOptions) + ->willThrowException(new \InvalidArgumentException('Some exception')); + + $this->objectManagerMock->expects($this->once()) + ->method('create') + ->willReturnMap([ + [\Magento\Catalog\Model\ResourceModel\Eav\Attribute::class, [], $this->attributeMock], + [\Magento\Eav\Model\Entity\Attribute\Set::class, [], $this->attributeSetMock] + ]); + + $this->attributeMock->expects($this->once()) + ->method('loadByCode') + ->willReturnSelf(); + $this->attributeSetMock->expects($this->never()) + ->method('setEntityTypeId') + ->willReturnSelf(); + $this->resultJsonFactoryMock->expects($this->once()) + ->method('create') + ->willReturn($this->resultJson); + $this->resultJson->expects($this->once()) + ->method('setJsonData') + ->with(json_encode([ + 'error' => true, + 'message' => $message, + ])) + ->willReturnSelf(); + + $this->getModel()->execute(); + } } diff --git a/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Product/AttributeTest.php b/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Product/AttributeTest.php index b85b03852b621..3a0b2b4bf7229 100644 --- a/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Product/AttributeTest.php +++ b/app/code/Magento/Catalog/Test/Unit/Controller/Adminhtml/Product/AttributeTest.php @@ -9,8 +9,9 @@ use Magento\Catalog\Controller\Adminhtml\Product\Attribute; use Magento\Framework\App\RequestInterface; use Magento\Framework\Cache\FrontendInterface; +use Magento\Framework\Message\ManagerInterface; use Magento\Framework\Registry; -use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; +use Magento\Framework\TestFramework\Unit\Helper\ObjectManager as ObjectManagerHelper; use Magento\Framework\View\Result\PageFactory; use Magento\Framework\Controller\ResultFactory; @@ -20,7 +21,7 @@ class AttributeTest extends \PHPUnit\Framework\TestCase { /** - * @var ObjectManager + * @var ObjectManagerHelper */ protected $objectManager; @@ -54,9 +55,14 @@ class AttributeTest extends \PHPUnit\Framework\TestCase */ protected $resultFactoryMock; + /** + * @var ManagerInterface|\PHPUnit_Framework_MockObject_MockObject + */ + private $messageManager; + protected function setUp() { - $this->objectManager = new ObjectManager($this); + $this->objectManager = new ObjectManagerHelper($this); $this->contextMock = $this->getMockBuilder(Context::class) ->disableOriginalConstructor() ->getMock(); @@ -74,6 +80,9 @@ protected function setUp() $this->resultFactoryMock = $this->getMockBuilder(ResultFactory::class) ->disableOriginalConstructor() ->getMock(); + $this->messageManager = $this->getMockBuilder(ManagerInterface::class) + ->disableOriginalConstructor() + ->getMock(); $this->contextMock->expects($this->any()) ->method('getRequest') @@ -81,6 +90,9 @@ protected function setUp() $this->contextMock->expects($this->any()) ->method('getResultFactory') ->willReturn($this->resultFactoryMock); + $this->contextMock->expects($this->once()) + ->method('getMessageManager') + ->willReturn($this->messageManager); } /** diff --git a/app/code/Magento/Catalog/view/adminhtml/templates/catalog/product/attribute/js.phtml b/app/code/Magento/Catalog/view/adminhtml/templates/catalog/product/attribute/js.phtml index 8a5f1919f78be..eeacc90fba914 100644 --- a/app/code/Magento/Catalog/view/adminhtml/templates/catalog/product/attribute/js.phtml +++ b/app/code/Magento/Catalog/view/adminhtml/templates/catalog/product/attribute/js.phtml @@ -40,13 +40,16 @@ function getFrontTab() { function checkOptionsPanelVisibility(){ if($('manage-options-panel')){ - var panel = $('manage-options-panel').up('.fieldset'); + var panel = $('manage-options-panel').up('.fieldset'), + activePanelClass = 'selected-type-options'; if($('frontend_input') && ($('frontend_input').value=='select' || $('frontend_input').value=='multiselect')){ panel.show(); + panel.addClass(activePanelClass); } else { panel.hide(); + panel.removeClass(activePanelClass); } } } diff --git a/app/code/Magento/Catalog/view/adminhtml/web/js/options.js b/app/code/Magento/Catalog/view/adminhtml/web/js/options.js index 6ea005915763c..1ec47f14fee88 100644 --- a/app/code/Magento/Catalog/view/adminhtml/web/js/options.js +++ b/app/code/Magento/Catalog/view/adminhtml/web/js/options.js @@ -20,7 +20,6 @@ define([ return function (config) { var optionPanel = jQuery('#manage-options-panel'), - optionsValues = [], editForm = jQuery('#edit_form'), attributeOption = { table: $('attribute-options-table'), @@ -145,7 +144,9 @@ define([ return optionDefaultInputType; } - }; + }, + tableBody = jQuery(), + activePanelClass = 'selected-type-options'; if ($('add_new_option_button')) { Event.observe('add_new_option_button', 'click', attributeOption.add.bind(attributeOption, {}, true)); @@ -180,30 +181,40 @@ define([ }); }); } - editForm.on('submit', function () { - optionPanel.find('input') - .each(function () { - if (this.disabled) { - return; - } + editForm.on('beforeSubmit', function () { + var optionsValues = [], + optionContainer = optionPanel.find('table tbody'); + + if (optionPanel.hasClass(activePanelClass)) { + optionContainer.find('input') + .each(function () { + if (this.disabled) { + return; + } - if (this.type === 'checkbox' || this.type === 'radio') { - if (this.checked) { + if (this.type === 'checkbox' || this.type === 'radio') { + if (this.checked) { + optionsValues.push(this.name + '=' + jQuery(this).val()); + } + } else { optionsValues.push(this.name + '=' + jQuery(this).val()); } - } else { - optionsValues.push(this.name + '=' + jQuery(this).val()); - } - }); - jQuery('') - .attr({ - type: 'hidden', - name: 'serialized_options' - }) - .val(JSON.stringify(optionsValues)) - .prependTo(editForm); - optionPanel.find('table') - .replaceWith(jQuery('
').text(jQuery.mage.__('Sending attribute values as package.'))); + }); + jQuery('') + .attr({ + type: 'hidden', + name: 'serialized_options' + }) + .val(JSON.stringify(optionsValues)) + .prependTo(editForm); + } + tableBody = optionContainer.detach(); + }); + editForm.on('afterValidate.error highlight.validate', function () { + if (optionPanel.hasClass(activePanelClass)) { + optionPanel.find('table').append(tableBody); + jQuery('input[name="serialized_options"]').remove(); + } }); window.attributeOption = attributeOption; window.optionDefaultInputType = attributeOption.getOptionInputType(); diff --git a/app/code/Magento/Swatches/Controller/Adminhtml/Product/Attribute/Plugin/Save.php b/app/code/Magento/Swatches/Controller/Adminhtml/Product/Attribute/Plugin/Save.php index 383c97a166d34..72d27152d639a 100644 --- a/app/code/Magento/Swatches/Controller/Adminhtml/Product/Attribute/Plugin/Save.php +++ b/app/code/Magento/Swatches/Controller/Adminhtml/Product/Attribute/Plugin/Save.php @@ -16,6 +16,8 @@ class Save { /** + * Performs the conversion of the frontend input value. + * * @param Attribute\Save $subject * @param RequestInterface $request * @return array @@ -26,15 +28,6 @@ public function beforeDispatch(Attribute\Save $subject, RequestInterface $reques $data = $request->getPostValue(); if (isset($data['frontend_input'])) { - //Data is serialized to overcome issues caused by max_input_vars value if it's modification is unavailable. - //See subject controller code and comments for more info. - if (isset($data['serialized_swatch_values']) - && in_array($data['frontend_input'], ['swatch_visual', 'swatch_text']) - ) { - $data['serialized_options'] = $data['serialized_swatch_values']; - unset($data['serialized_swatch_values']); - } - switch ($data['frontend_input']) { case 'swatch_visual': $data[Swatch::SWATCH_INPUT_TYPE_KEY] = Swatch::SWATCH_INPUT_TYPE_VISUAL; diff --git a/app/code/Magento/Swatches/Test/Mftf/ActionGroup/ColorPickerActionGroup.xml b/app/code/Magento/Swatches/Test/Mftf/ActionGroup/ColorPickerActionGroup.xml new file mode 100644 index 0000000000000..1bdb6d688656f --- /dev/null +++ b/app/code/Magento/Swatches/Test/Mftf/ActionGroup/ColorPickerActionGroup.xml @@ -0,0 +1,29 @@ + + + + + + + + + + + + + + + + + + + + diff --git a/app/code/Magento/Swatches/Test/Mftf/Data/SwatchAttributeData.xml b/app/code/Magento/Swatches/Test/Mftf/Data/SwatchAttributeData.xml index 7822120337e1d..c2e0cce712889 100644 --- a/app/code/Magento/Swatches/Test/Mftf/Data/SwatchAttributeData.xml +++ b/app/code/Magento/Swatches/Test/Mftf/Data/SwatchAttributeData.xml @@ -11,5 +11,6 @@ VisualSwatchAttr Visual Swatch + visual_swatch diff --git a/app/code/Magento/Swatches/Test/Mftf/Page/AdminProductAttributeFormPage.xml b/app/code/Magento/Swatches/Test/Mftf/Page/AdminProductAttributeFormPage.xml new file mode 100644 index 0000000000000..efad7e7b3578b --- /dev/null +++ b/app/code/Magento/Swatches/Test/Mftf/Page/AdminProductAttributeFormPage.xml @@ -0,0 +1,14 @@ + + + + +
+
+ + diff --git a/app/code/Magento/Swatches/Test/Mftf/Section/AdminColorPickerSection.xml b/app/code/Magento/Swatches/Test/Mftf/Section/AdminColorPickerSection.xml new file mode 100644 index 0000000000000..16c38b9fc9e36 --- /dev/null +++ b/app/code/Magento/Swatches/Test/Mftf/Section/AdminColorPickerSection.xml @@ -0,0 +1,15 @@ + + + + +
+ + +
+ diff --git a/app/code/Magento/Swatches/Test/Mftf/Section/AdminManageSwatchSection.xml b/app/code/Magento/Swatches/Test/Mftf/Section/AdminManageSwatchSection.xml new file mode 100644 index 0000000000000..01d6dd99418ec --- /dev/null +++ b/app/code/Magento/Swatches/Test/Mftf/Section/AdminManageSwatchSection.xml @@ -0,0 +1,16 @@ + + + + +
+ + + +
+ diff --git a/app/code/Magento/Swatches/Test/Mftf/Test/AdminCreateVisualSwatchWithNonValidOptionsTest.xml b/app/code/Magento/Swatches/Test/Mftf/Test/AdminCreateVisualSwatchWithNonValidOptionsTest.xml new file mode 100644 index 0000000000000..433eb9e8be02e --- /dev/null +++ b/app/code/Magento/Swatches/Test/Mftf/Test/AdminCreateVisualSwatchWithNonValidOptionsTest.xml @@ -0,0 +1,75 @@ + + + + + + + + + <description value="Admin should be able to create swatch product attribute"/> + <severity value="BLOCKER"/> + <testCaseId value="MAGETWO-95487"/> + <group value="swatches"/> + </annotations> + <before> + <actionGroup ref="LoginAsAdmin" stepKey="login"/> + </before> + <after> + <!-- Remove attribute --> + <actionGroup ref="deleteProductAttribute" stepKey="deleteProductAttribute"> + <argument name="ProductAttribute" value="VisualSwatchAttribute"/> + </actionGroup> + + <actionGroup ref="logout" stepKey="logout"/> + </after> + + <amOnPage url="{{ProductAttributePage.url}}" stepKey="navigateToNewProductAttributePage"/> + <waitForPageLoad stepKey="waitForPageLoad"/> + + <!-- Set attribute properties --> + <fillField selector="{{AttributePropertiesSection.defaultLabel}}" + userInput="{{VisualSwatchAttribute.default_label}}" stepKey="fillDefaultLabel"/> + <selectOption selector="{{AttributePropertiesSection.inputType}}" + userInput="{{VisualSwatchAttribute.input_type}}" stepKey="fillInputType"/> + + <!-- Set advanced attribute properties --> + <click selector="{{AdvancedAttributePropertiesSection.advancedAttributePropertiesSectionToggle}}" + stepKey="showAdvancedAttributePropertiesSection"/> + <waitForElementVisible selector="{{AdvancedAttributePropertiesSection.attributeCode}}" + stepKey="waitForSlideOut"/> + <fillField selector="{{AdvancedAttributePropertiesSection.attributeCode}}" + userInput="{{VisualSwatchAttribute.attribute_code}}" + stepKey="fillAttributeCode"/> + + <!-- Add new swatch option without label --> + <click selector="{{AdminManageSwatchSection.addSwatch}}" stepKey="clickAddSwatch"/> + <actionGroup ref="openSwatchMenuByIndex" stepKey="clickSwatch"> + <argument name="index" value="0"/> + </actionGroup> + <click selector="{{AdminManageSwatchSection.chooseColorRow('1')}}" stepKey="clickChooseColor"/> + <actionGroup ref="setColorPickerValueByHex" stepKey="fillHex"> + <argument name="hexColor" value="ff0000"/> + </actionGroup> + + <!-- Scroll to top of the page --> + <scrollToTopOfPage stepKey="scrollToTop"/> + + <!-- Save the new product attribute --> + <click selector="{{AttributePropertiesSection.Save}}" stepKey="clickSave1"/> + <waitForElementVisible selector="{{AdminMessagesSection.error}}" stepKey="waitForError"/> + + <!-- Fill options data --> + <fillField selector="{{AdminManageSwatchSection.adminInputByIndex('0')}}" + userInput="red" stepKey="fillAdmin"/> + + <!-- Save the new product attribute --> + <click selector="{{AttributePropertiesSection.Save}}" stepKey="clickSave2"/> + <waitForElementVisible selector="{{AdminMessagesSection.successMessage}}" + stepKey="waitForSuccessMessage"/> + </test> +</tests> diff --git a/app/code/Magento/Swatches/view/adminhtml/web/js/product-attributes.js b/app/code/Magento/Swatches/view/adminhtml/web/js/product-attributes.js index 1a58e4b6f2e7a..6812060685792 100644 --- a/app/code/Magento/Swatches/view/adminhtml/web/js/product-attributes.js +++ b/app/code/Magento/Swatches/view/adminhtml/web/js/product-attributes.js @@ -16,7 +16,8 @@ define([ 'use strict'; return function (optionConfig) { - var swatchProductAttributes = { + var activePanelClass = 'selected-type-options', + swatchProductAttributes = { frontendInput: $('#frontend_input'), isFilterable: $('#is_filterable'), isFilterableInSearch: $('#is_filterable_in_search'), @@ -337,6 +338,7 @@ define([ */ _showPanel: function (el) { el.closest('.fieldset').show(); + el.addClass(activePanelClass); this._render(el.attr('id')); }, @@ -346,6 +348,7 @@ define([ */ _hidePanel: function (el) { el.closest('.fieldset').hide(); + el.removeClass(activePanelClass); }, /** @@ -413,7 +416,11 @@ define([ }; $(function () { - var editForm = $('#edit_form'); + var editForm = $('#edit_form'), + swatchVisualPanel = $('#swatch-visual-options-panel'), + swatchTextPanel = $('#swatch-text-options-panel'), + tableBody = $(), + activePanel = $(); $('#frontend_input').bind('change', function () { swatchProductAttributes.bindAttributeInputType(); @@ -429,30 +436,35 @@ define([ .collapsable() .collapse('hide'); - editForm.on('submit', function () { - var activePanel, - swatchValues = [], - swatchVisualPanel = $('#swatch-visual-options-panel'), - swatchTextPanel = $('#swatch-text-options-panel'); + editForm.on('beforeSubmit', function () { + var swatchValues = [], + optionContainer; - activePanel = swatchTextPanel.is(':visible') ? swatchTextPanel : swatchVisualPanel; + activePanel = swatchTextPanel.hasClass(activePanelClass) ? swatchTextPanel : swatchVisualPanel; + optionContainer = activePanel.find('table tbody'); - activePanel.find('table input') - .each(function () { - swatchValues.push(this.name + '=' + $(this).val()); - }); + if (activePanel.hasClass(activePanelClass)) { + optionContainer + .find('input') + .each(function () { + swatchValues.push(this.name + '=' + $(this).val()); + }); - $('<input>').attr({ + $('<input>').attr({ type: 'hidden', - name: 'serialized_swatch_values' + name: 'serialized_options' }) - .val(JSON.stringify(swatchValues)) - .prependTo(editForm); + .val(JSON.stringify(swatchValues)) + .prependTo(editForm); + } - [swatchVisualPanel, swatchTextPanel].forEach(function (el) { - $(el).find('table') - .replaceWith($('<div>').text($.mage.__('Sending swatch values as package.'))); - }); + tableBody = optionContainer.detach(); + }); + editForm.on('afterValidate.error highlight.validate', function () { + if (activePanel.hasClass(activePanelClass)) { + activePanel.find('table').append(tableBody); + $('input[name="serialized_options"]').remove(); + } }); }); diff --git a/dev/tests/acceptance/.gitignore b/dev/tests/acceptance/.gitignore index 7e27d5178fc48..9ca88b3597bad 100755 --- a/dev/tests/acceptance/.gitignore +++ b/dev/tests/acceptance/.gitignore @@ -6,5 +6,6 @@ tests/_output/* tests/functional.suite.yml tests/functional/Magento/FunctionalTest/_generated vendor/* -mftf.logm +mftf.log +/.credentials.example /utils/ diff --git a/dev/tests/functional/tests/app/Magento/Catalog/Test/Handler/CatalogProductAttribute/Curl.php b/dev/tests/functional/tests/app/Magento/Catalog/Test/Handler/CatalogProductAttribute/Curl.php index f1086f4871f3b..59faeda78aa5e 100644 --- a/dev/tests/functional/tests/app/Magento/Catalog/Test/Handler/CatalogProductAttribute/Curl.php +++ b/dev/tests/functional/tests/app/Magento/Catalog/Test/Handler/CatalogProductAttribute/Curl.php @@ -134,11 +134,71 @@ public function persist(FixtureInterface $fixture = null) } /** + * Additional data handling. + * * @param array $data * @return array */ protected function changeStructureOfTheData(array $data) { + $serializedOptions = $this->getSerializeOptions($data, ['option']); + if ($serializedOptions) { + $data['serialized_options'] = $serializedOptions; + unset($data['option']); + } + return $data; } + + /** + * Provides serialized product attribute options. + * + * @param array $data + * @param array $optionKeys + * @return string + */ + protected function getSerializeOptions(array $data, array $optionKeys): string + { + $options = []; + foreach ($optionKeys as $optionKey) { + if (!empty($data[$optionKey])) { + $options = array_merge( + $options, + $this->getEncodedOptions([$optionKey => $data[$optionKey]]) + ); + } + } + + return json_encode($options); + } + + /** + * Provides encoded attribute values. + * + * @param array $data + * @return array + */ + private function getEncodedOptions(array $data): array + { + $optionsData = []; + $iterator = new \RecursiveIteratorIterator(new \RecursiveArrayIterator($data)); + + foreach ($iterator as $value) { + $depth = $iterator->getDepth(); + $option = ''; + $level = 0; + $option .= $iterator->getSubIterator($level)->key(); + $level++; + + while ($level <= $depth) { + $option .= '[' . $iterator->getSubIterator($level)->key() . ']'; + $level++; + } + + $option .= '=' . $value; + $optionsData[] = $option; + } + + return $optionsData; + } } diff --git a/dev/tests/functional/tests/app/Magento/Swatches/Test/Handler/SwatchProductAttribute/Curl.php b/dev/tests/functional/tests/app/Magento/Swatches/Test/Handler/SwatchProductAttribute/Curl.php index 083fa246c96ef..5e4a5cc45fe69 100644 --- a/dev/tests/functional/tests/app/Magento/Swatches/Test/Handler/SwatchProductAttribute/Curl.php +++ b/dev/tests/functional/tests/app/Magento/Swatches/Test/Handler/SwatchProductAttribute/Curl.php @@ -38,12 +38,14 @@ public function __construct(DataInterface $configuration, EventManagerInterface */ protected function changeStructureOfTheData(array $data) { - $data = parent::changeStructureOfTheData($data); $data['optiontext'] = $data['option']; $data['swatchtext'] = [ 'value' => $data['option']['value'] ]; + $data['serialized_options'] = $this->getSerializeOptions($data, ['optiontext', 'swatchtext']); unset($data['option']); + $data = parent::changeStructureOfTheData($data); + return $data; } } diff --git a/dev/tests/integration/testsuite/Magento/Swatches/Controller/Adminhtml/Product/AttributeTest.php b/dev/tests/integration/testsuite/Magento/Swatches/Controller/Adminhtml/Product/AttributeTest.php index be9fd96d75892..09fc55256121b 100644 --- a/dev/tests/integration/testsuite/Magento/Swatches/Controller/Adminhtml/Product/AttributeTest.php +++ b/dev/tests/integration/testsuite/Magento/Swatches/Controller/Adminhtml/Product/AttributeTest.php @@ -7,6 +7,8 @@ namespace Magento\Swatches\Controller\Adminhtml\Product; +use Magento\Framework\App\Request\Http as HttpRequest; +use Magento\Framework\Data\Form\FormKey; use Magento\Framework\Exception\LocalizedException; /** @@ -17,6 +19,21 @@ */ class AttributeTest extends \Magento\TestFramework\TestCase\AbstractBackendController { + /** + * @var FormKey + */ + private $formKey; + + /** + * @inheritDoc + */ + protected function setUp() + { + parent::setUp(); + + $this->formKey = $this->_objectManager->get(FormKey::class); + } + /** * Generate random hex color. * @@ -48,12 +65,15 @@ private function getSwatchVisualDataSet(int $optionsCount): array $optionsData []= "optionvisual[value][option_{$i}][1]={$expectedOptionLabelOnStoreView}"; $optionsData []= "optionvisual[delete][option_{$i}]="; } - $optionsData []= "visual_swatch_validation="; - $optionsData []= "visual_swatch_validation_unique="; + return [ 'attribute_data' => array_merge_recursive( [ - 'serialized_swatch_values' => json_encode($optionsData), + 'serialized_options' => json_encode($optionsData), + ], + [ + 'visual_swatch_validation' => '', + 'visual_swatch_validation_unique' => '', ], $this->getAttributePreset(), [ @@ -86,12 +106,15 @@ private function getSwatchTextDataSet(int $optionsCount): array $optionsData []= "optiontext[value][option_{$i}][1]={$expectedOptionLabelOnStoreView}"; $optionsData []= "optiontext[delete][option_{$i}]="; } - $optionsData []= "text_swatch_validation="; - $optionsData []= "text_swatch_validation_unique="; + return [ 'attribute_data' => array_merge_recursive( [ - 'serialized_swatch_values' => json_encode($optionsData), + 'serialized_options' => json_encode($optionsData), + ], + [ + 'text_swatch_validation' => '', + 'text_swatch_validation_unique' => '', ], $this->getAttributePreset(), [ @@ -111,7 +134,6 @@ private function getSwatchTextDataSet(int $optionsCount): array private function getAttributePreset(): array { return [ - 'serialized_options' => '[]', 'form_key' => 'XxtpPYjm2YPYUlAt', 'frontend_label' => [ 0 => 'asdasd', @@ -176,7 +198,9 @@ public function testLargeOptionsDataSet( int $expectedOptionsCount, array $expectedLabels ) { + $this->getRequest()->setMethod(HttpRequest::METHOD_POST); $this->getRequest()->setPostValue($attributeData); + $this->getRequest()->setPostValue('form_key', $this->formKey->getFormKey()); $this->dispatch('backend/catalog/product_attribute/save'); $entityTypeId = $this->_objectManager->create( \Magento\Eav\Model\Entity::class diff --git a/lib/internal/Magento/Framework/Serialize/Serializer/FormData.php b/lib/internal/Magento/Framework/Serialize/Serializer/FormData.php new file mode 100644 index 0000000000000..077e91797d2b5 --- /dev/null +++ b/lib/internal/Magento/Framework/Serialize/Serializer/FormData.php @@ -0,0 +1,55 @@ +<?php +/** + * Copyright © Magento, Inc. All rights reserved. + * See COPYING.txt for license details. + */ + +declare(strict_types=1); + +namespace Magento\Framework\Serialize\Serializer; + +use Magento\Framework\Serialize\Serializer\Json; + +/** + * Class for processing of serialized form data. + */ +class FormData +{ + /** + * @var Json + */ + private $serializer; + + /** + * @param Json $serializer + */ + public function __construct(Json $serializer) + { + $this->serializer = $serializer; + } + + /** + * Provides form data from the serialized data. + * + * @param string $serializedData + * @return array + * @throws \InvalidArgumentException + */ + public function unserialize(string $serializedData): array + { + $encodedFields = $this->serializer->unserialize($serializedData); + + if (!is_array($encodedFields)) { + throw new \InvalidArgumentException('Unable to unserialize value.'); + } + + $formData = []; + foreach ($encodedFields as $item) { + $decodedFieldData = []; + parse_str($item, $decodedFieldData); + $formData = array_replace_recursive($formData, $decodedFieldData); + } + + return $formData; + } +} diff --git a/lib/internal/Magento/Framework/Serialize/Test/Unit/Serializer/FormDataTest.php b/lib/internal/Magento/Framework/Serialize/Test/Unit/Serializer/FormDataTest.php new file mode 100644 index 0000000000000..7729a2da97efb --- /dev/null +++ b/lib/internal/Magento/Framework/Serialize/Test/Unit/Serializer/FormDataTest.php @@ -0,0 +1,106 @@ +<?php +/** + * Copyright © Magento, Inc. All rights reserved. + * See COPYING.txt for license details. + */ + +namespace Magento\Framework\Serialize\Test\Unit\Serializer; + +use Magento\Framework\Serialize\Serializer\FormData; +use Magento\Framework\Serialize\Serializer\Json; +use Psr\Log\InvalidArgumentException; + +/** + * Test for Magento\Framework\Serialize\Serializer\FormData class. + */ +class FormDataTest extends \PHPUnit\Framework\TestCase +{ + /** + * @var Json|\PHPUnit_Framework_MockObject_MockObject + */ + private $jsonSerializerMock; + + /** + * @var FormData + */ + private $formDataSerializer; + + /** + * @inheritdoc + */ + protected function setUp() + { + $this->jsonSerializerMock = $this->createMock(Json::class); + $this->formDataSerializer = new FormData($this->jsonSerializerMock); + } + + /** + * @param string $serializedData + * @param array $encodedFields + * @param array $expectedFormData + * @return void + * @dataProvider unserializeDataProvider + */ + public function testUnserialize(string $serializedData, array $encodedFields, array $expectedFormData) + { + $this->jsonSerializerMock->expects($this->once()) + ->method('unserialize') + ->with($serializedData) + ->willReturn($encodedFields); + + $this->assertEquals($expectedFormData, $this->formDataSerializer->unserialize($serializedData)); + } + + /** + * @return array + */ + public function unserializeDataProvider(): array + { + return [ + [ + 'serializedData' => + '["option[order][option_0]=1","option[value][option_0]=1","option[delete][option_0]="]', + 'encodedFields' => [ + 'option[order][option_0]=1', + 'option[value][option_0]=1', + 'option[delete][option_0]=', + ], + 'expectedFormData' => [ + 'option' => [ + 'order' => [ + 'option_0' => '1', + ], + 'value' => [ + 'option_0' => '1', + ], + 'delete' => [ + 'option_0' => '', + ], + ], + ], + ], + [ + 'serializedData' => '[]', + 'encodedFields' => [], + 'expectedFormData' => [], + ], + ]; + } + + /** + * @return void + * @expectedException InvalidArgumentException + * @expectedExceptionMessage Unable to unserialize value. + */ + public function testUnserializeWithWrongSerializedData() + { + $serializedData = 'test'; + + $this->jsonSerializerMock->expects($this->once()) + ->method('unserialize') + ->with($serializedData) + ->willReturn('test'); + + $this->formDataSerializer->unserialize($serializedData); + } +} diff --git a/lib/web/mage/backend/validation.js b/lib/web/mage/backend/validation.js index d3ab7dd086a43..f141fb3eeb8d0 100644 --- a/lib/web/mage/backend/validation.js +++ b/lib/web/mage/backend/validation.js @@ -171,6 +171,7 @@ this._submit(); } else { this._showErrors(response); + $(this.element[0]).trigger('afterValidate.error'); $('body').trigger('processStop'); } }, diff --git a/setup/pub/magento/setup/app.js b/setup/pub/magento/setup/app.js index b433e3f19c8bf..f0abd15d106c2 100644 --- a/setup/pub/magento/setup/app.js +++ b/setup/pub/magento/setup/app.js @@ -56,6 +56,9 @@ app.config(['$httpProvider', '$stateProvider', function ($httpProvider, $statePr return $delegate; }); }) + .config(['$locationProvider', function($locationProvider) { + $locationProvider.hashPrefix(''); + }]) .run(function ($rootScope, $state) { $rootScope.$state = $state; });