From bce7f54ec285f1414a652cf71797c09959e115f8 Mon Sep 17 00:00:00 2001 From: "o.kravchuk" Date: Fri, 14 Aug 2026 08:53:17 +0300 Subject: [PATCH] magento/magento2#33055: Validate filterable attribute input types on admin save Reject Use in Layered Navigation for catalog input types the admin form does not allow, using the same allowlist for admin save and REST. --- .../Adminhtml/Product/Attribute/Save.php | 101 +++++++------- .../Attribute/FilterableAllowedInputTypes.php | 38 +++++ .../Model/Product/Attribute/Repository.php | 96 +++++-------- .../Adminhtml/Product/Attribute/SaveTest.php | 132 +++++++++++++++++- .../FilterableAllowedInputTypesTest.php | 47 +++++++ .../Product/Attribute/RepositoryTest.php | 21 ++- app/code/Magento/Catalog/etc/di.xml | 10 ++ .../Save/FilterableInputTypeTest.php | 120 ++++++++++++++++ .../Product/Attribute/RepositoryTest.php | 94 +++++++++++++ 9 files changed, 547 insertions(+), 112 deletions(-) create mode 100644 app/code/Magento/Catalog/Model/Product/Attribute/FilterableAllowedInputTypes.php create mode 100644 app/code/Magento/Catalog/Test/Unit/Model/Product/Attribute/FilterableAllowedInputTypesTest.php create mode 100644 dev/tests/integration/testsuite/Magento/Catalog/Controller/Adminhtml/Product/Attribute/Save/FilterableInputTypeTest.php 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 ca80261a467d3..cc469654ccd23 100644 --- a/app/code/Magento/Catalog/Controller/Adminhtml/Product/Attribute/Save.php +++ b/app/code/Magento/Catalog/Controller/Adminhtml/Product/Attribute/Save.php @@ -12,6 +12,7 @@ use Magento\Catalog\Api\Data\ProductAttributeInterface; use Magento\Catalog\Controller\Adminhtml\Product\Attribute; use Magento\Catalog\Helper\Product; +use Magento\Catalog\Model\Product\Attribute\FilterableAllowedInputTypes; use Magento\Catalog\Model\Product\Attribute\Frontend\Inputtype\Presentation; use Magento\Framework\Serialize\Serializer\FormData; use Magento\Catalog\Model\Product\AttributeSet\BuildFactory; @@ -38,41 +39,6 @@ */ class Save extends Attribute implements HttpPostActionInterface { - /** - * @var BuildFactory - */ - protected $buildFactory; - - /** - * @var FilterManager - */ - protected $filterManager; - - /** - * @var Product - */ - protected $productHelper; - - /** - * @var AttributeFactory - */ - protected $attributeFactory; - - /** - * @var ValidatorFactory - */ - protected $validatorFactory; - - /** - * @var CollectionFactory - */ - protected $groupCollectionFactory; - - /** - * @var LayoutFactory - */ - private $layoutFactory; - /** * @var Presentation */ @@ -83,6 +49,11 @@ class Save extends Attribute implements HttpPostActionInterface */ private $formDataSerializer; + /** + * @var FilterableAllowedInputTypes + */ + private $filterableAllowedInputTypes; + /** * @param Context $context * @param FrontendInterface $attributeLabelCache @@ -97,6 +68,7 @@ class Save extends Attribute implements HttpPostActionInterface * @param LayoutFactory $layoutFactory * @param Presentation|null $presentation * @param FormData|null $formDataSerializer + * @param FilterableAllowedInputTypes|null $filterableAllowedInputTypes * @SuppressWarnings(PHPMD.ExcessiveParameterList) */ public function __construct( @@ -104,27 +76,23 @@ public function __construct( FrontendInterface $attributeLabelCache, Registry $coreRegistry, PageFactory $resultPageFactory, - BuildFactory $buildFactory, - AttributeFactory $attributeFactory, - ValidatorFactory $validatorFactory, - CollectionFactory $groupCollectionFactory, - FilterManager $filterManager, - Product $productHelper, - LayoutFactory $layoutFactory, + protected BuildFactory $buildFactory, + protected AttributeFactory $attributeFactory, + protected ValidatorFactory $validatorFactory, + protected CollectionFactory $groupCollectionFactory, + protected FilterManager $filterManager, + protected Product $productHelper, + private LayoutFactory $layoutFactory, ?Presentation $presentation = null, - ?FormData $formDataSerializer = null + ?FormData $formDataSerializer = null, + ?FilterableAllowedInputTypes $filterableAllowedInputTypes = null ) { parent::__construct($context, $attributeLabelCache, $coreRegistry, $resultPageFactory); - $this->buildFactory = $buildFactory; - $this->filterManager = $filterManager; - $this->productHelper = $productHelper; - $this->attributeFactory = $attributeFactory; - $this->validatorFactory = $validatorFactory; - $this->groupCollectionFactory = $groupCollectionFactory; - $this->layoutFactory = $layoutFactory; $this->presentation = $presentation ?: ObjectManager::getInstance()->get(Presentation::class); $this->formDataSerializer = $formDataSerializer ?: ObjectManager::getInstance()->get(FormData::class); + $this->filterableAllowedInputTypes = $filterableAllowedInputTypes + ?: ObjectManager::getInstance()->get(FilterableAllowedInputTypes::class); } /** @@ -254,6 +222,11 @@ public function execute() $data += ['is_filterable' => 0, 'is_filterable_in_search' => 0]; + $filterableValidationResult = $this->validateFilterableFlags($data, $attributeId); + if ($filterableValidationResult) { + return $filterableValidationResult; + } + $defaultValueField = $model->getDefaultValueByInput($data['frontend_input']); if ($defaultValueField) { $data['default_value'] = $this->getRequest()->getParam($defaultValueField); @@ -356,6 +329,34 @@ public function execute() return $this->returnResult('catalog/*/', [], ['error' => true]); } + /** + * Reject layered-navigation flags for input types the admin form does not allow. + * + * @param array $data + * @param mixed $attributeId + * @return Json|Redirect|null + */ + private function validateFilterableFlags(array $data, $attributeId) + { + if ($this->filterableAllowedInputTypes->isAllowed($data['frontend_input'] ?? null)) { + return null; + } + + if ((int)($data['is_filterable'] ?? 0) || (int)($data['is_filterable_in_search'] ?? 0)) { + $this->messageManager->addErrorMessage( + __('Can be used only with catalog input type Yes/No, Dropdown, Multiple Select and Price.') + ); + $this->_session->setAttributeData($data); + return $this->returnResult( + 'catalog/*/edit', + ['attribute_id' => $attributeId, '_current' => true], + ['error' => true] + ); + } + + return null; + } + /** * Provides an initialized Result object. * diff --git a/app/code/Magento/Catalog/Model/Product/Attribute/FilterableAllowedInputTypes.php b/app/code/Magento/Catalog/Model/Product/Attribute/FilterableAllowedInputTypes.php new file mode 100644 index 0000000000000..02f9ea53b2038 --- /dev/null +++ b/app/code/Magento/Catalog/Model/Product/Attribute/FilterableAllowedInputTypes.php @@ -0,0 +1,38 @@ +inputTypes = $inputTypes; + } + + /** + * Check whether the catalog input type may be used in layered navigation. + * + * @param mixed $frontendInput + * @return bool + */ + public function isAllowed(mixed $frontendInput): bool + { + return in_array((string)$frontendInput, $this->inputTypes, true); + } +} diff --git a/app/code/Magento/Catalog/Model/Product/Attribute/Repository.php b/app/code/Magento/Catalog/Model/Product/Attribute/Repository.php index 89aeb40c232b5..9f37fda5e0c3c 100644 --- a/app/code/Magento/Catalog/Model/Product/Attribute/Repository.php +++ b/app/code/Magento/Catalog/Model/Product/Attribute/Repository.php @@ -7,9 +7,18 @@ use Laminas\Validator\Regex; use Magento\Catalog\Api\Data\EavAttributeInterface; +use Magento\Catalog\Helper\Product; +use Magento\Catalog\Model\ResourceModel\Attribute as AttributeResource; +use Magento\Eav\Api\AttributeRepositoryInterface; +use Magento\Eav\Model\Adminhtml\System\Config\Source\Inputtype\ValidatorFactory; +use Magento\Eav\Model\Config; use Magento\Eav\Model\Entity\Attribute; +use Magento\Eav\Model\Validator\Attribute\Code; +use Magento\Framework\Api\SearchCriteriaBuilder; +use Magento\Framework\App\ObjectManager; use Magento\Framework\Exception\InputException; use Magento\Framework\Exception\NoSuchEntityException; +use Magento\Framework\Filter\FilterManager; /** * Product attribute repository @@ -18,76 +27,43 @@ */ class Repository implements \Magento\Catalog\Api\ProductAttributeRepositoryInterface { - private const FILTERABLE_ALLOWED_INPUT_TYPES = ['date', 'datetime', 'text', 'textarea', 'texteditor']; - - /** - * @var \Magento\Catalog\Model\ResourceModel\Attribute - */ - protected $attributeResource; - - /** - * @var \Magento\Eav\Model\AttributeRepository - */ - protected $eavAttributeRepository; - - /** - * @var \Magento\Eav\Model\Config - */ - protected $eavConfig; - /** - * @var \Magento\Eav\Model\Adminhtml\System\Config\Source\Inputtype\ValidatorFactory + * @var ValidatorFactory + * @deprecated + * @see $validatorFactory */ protected $inputtypeValidatorFactory; /** - * @var \Magento\Catalog\Helper\Product - */ - protected $productHelper; - - /** - * @var \Magento\Framework\Filter\FilterManager - */ - protected $filterManager; - - /** - * @var \Magento\Framework\Api\SearchCriteriaBuilder - */ - protected $searchCriteriaBuilder; - - /** - * @var \Magento\Eav\Model\Validator\Attribute\Code + * @var FilterableAllowedInputTypes */ - protected $attributeCodeValidator; + private FilterableAllowedInputTypes $filterableAllowedInputTypes; /** - * @param \Magento\Catalog\Model\ResourceModel\Attribute $attributeResource - * @param \Magento\Catalog\Helper\Product $productHelper - * @param \Magento\Framework\Filter\FilterManager $filterManager - * @param \Magento\Eav\Api\AttributeRepositoryInterface $eavAttributeRepository - * @param \Magento\Eav\Model\Config $eavConfig - * @param \Magento\Eav\Model\Adminhtml\System\Config\Source\Inputtype\ValidatorFactory $validatorFactory - * @param \Magento\Framework\Api\SearchCriteriaBuilder $searchCriteriaBuilder - * @param \Magento\Eav\Model\Validator\Attribute\Code $attributeCodeValidator + * @param AttributeResource $attributeResource + * @param Product $productHelper + * @param FilterManager $filterManager + * @param AttributeRepositoryInterface $eavAttributeRepository + * @param Config $eavConfig + * @param ValidatorFactory $validatorFactory + * @param SearchCriteriaBuilder $searchCriteriaBuilder + * @param Code $attributeCodeValidator + * @param FilterableAllowedInputTypes|null $filterableAllowedInputTypes */ public function __construct( - \Magento\Catalog\Model\ResourceModel\Attribute $attributeResource, - \Magento\Catalog\Helper\Product $productHelper, - \Magento\Framework\Filter\FilterManager $filterManager, - \Magento\Eav\Api\AttributeRepositoryInterface $eavAttributeRepository, - \Magento\Eav\Model\Config $eavConfig, - \Magento\Eav\Model\Adminhtml\System\Config\Source\Inputtype\ValidatorFactory $validatorFactory, - \Magento\Framework\Api\SearchCriteriaBuilder $searchCriteriaBuilder, - \Magento\Eav\Model\Validator\Attribute\Code $attributeCodeValidator + protected AttributeResource $attributeResource, + protected Product $productHelper, + protected FilterManager $filterManager, + protected AttributeRepositoryInterface $eavAttributeRepository, + protected Config $eavConfig, + protected ValidatorFactory $validatorFactory, + protected SearchCriteriaBuilder $searchCriteriaBuilder, + protected Code $attributeCodeValidator, + ?FilterableAllowedInputTypes $filterableAllowedInputTypes = null ) { - $this->attributeResource = $attributeResource; - $this->productHelper = $productHelper; - $this->filterManager = $filterManager; - $this->eavAttributeRepository = $eavAttributeRepository; - $this->eavConfig = $eavConfig; $this->inputtypeValidatorFactory = $validatorFactory; - $this->searchCriteriaBuilder = $searchCriteriaBuilder; - $this->attributeCodeValidator = $attributeCodeValidator; + $this->filterableAllowedInputTypes = $filterableAllowedInputTypes + ?? ObjectManager::getInstance()->get(FilterableAllowedInputTypes::class); } /** @@ -120,7 +96,7 @@ public function getList(\Magento\Framework\Api\SearchCriteriaInterface $searchCr */ public function save(\Magento\Catalog\Api\Data\ProductAttributeInterface $attribute) { - if (in_array($attribute->getFrontendInput(), self::FILTERABLE_ALLOWED_INPUT_TYPES)) { + if (!$this->filterableAllowedInputTypes->isAllowed($attribute->getFrontendInput())) { if ($attribute->getIsFilterable()) { throw InputException::invalidFieldValue( EavAttributeInterface::IS_FILTERABLE, @@ -312,7 +288,7 @@ protected function validateCode($code) protected function validateFrontendInput($frontendInput) { /** @var \Magento\Eav\Model\Adminhtml\System\Config\Source\Inputtype\Validator $validator */ - $validator = $this->inputtypeValidatorFactory->create(); + $validator = $this->validatorFactory->create(); if (!$validator->isValid($frontendInput)) { throw InputException::invalidFieldValue('frontend_input', $frontendInput); } 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 4367ba30d8623..0b01e1227ca2f 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 @@ -12,6 +12,7 @@ use Magento\Catalog\Api\Data\ProductAttributeInterface; use Magento\Catalog\Controller\Adminhtml\Product\Attribute\Save; use Magento\Catalog\Helper\Product as ProductHelper; +use Magento\Catalog\Model\Product\Attribute\FilterableAllowedInputTypes; use Magento\Catalog\Model\Entity\Attribute; use Magento\Catalog\Model\Product\Attribute\Frontend\Inputtype\Presentation; use Magento\Catalog\Model\Product\AttributeSet\Build; @@ -134,9 +135,13 @@ class SaveTest extends AttributeTest /** * @var Session|MockObject */ - private $sessionMock; + /** + * @var FilterableAllowedInputTypes + */ + private $filterableAllowedInputTypes; + protected function setUp(): void { parent::setUp(); @@ -157,6 +162,9 @@ protected function setUp(): void $this->redirectMock = $this->createMock(ResultRedirect::class); $this->jsonResultMock = $this->createMock(ResultJson::class); $this->productAttributeMock = $this->createMock(Attribute::class); + $this->filterableAllowedInputTypes = new FilterableAllowedInputTypes( + ['boolean', 'select', 'multiselect', 'price'] + ); $this->buildFactoryMock->expects($this->any()) ->method('create') @@ -189,6 +197,7 @@ protected function getModel() 'formDataSerializer' => $this->formDataSerializerMock, 'attributeCodeValidator' => $this->attributeCodeValidatorMock, 'presentation' => $this->presentationMock, + 'filterableAllowedInputTypes' => $this->filterableAllowedInputTypes, '_session' => $this->sessionMock ]); } @@ -259,6 +268,9 @@ public function testConstructorFallbackUsesGlobalObjectManagerForFormDataSeriali if ($type === Presentation::class) { return $this->presentationMock; } + if ($type === FilterableAllowedInputTypes::class) { + return $this->filterableAllowedInputTypes; + } return null; }); ObjectManager::setInstance($objectManagerMock); @@ -279,6 +291,7 @@ public function testConstructorFallbackUsesGlobalObjectManagerForFormDataSeriali // Intentionally omit 'formDataSerializer' to trigger fallback 'formDataSerializer' => null, 'presentation' => $this->presentationMock, + 'filterableAllowedInputTypes' => $this->filterableAllowedInputTypes, '_session' => $this->sessionMock ]); @@ -631,6 +644,7 @@ public function testEntityTypeCheckFailsWhenBackendModelProvided() 'layoutFactory' => $this->layoutFactoryMock, 'formDataSerializer' => $this->formDataSerializerMock, 'presentation' => $this->presentationMock, + 'filterableAllowedInputTypes' => $this->filterableAllowedInputTypes, '_session' => $this->sessionMock ]); @@ -960,6 +974,121 @@ public function testNewAttributeSetGenericExceptionAddsExceptionMessage() $this->assertInstanceOf(ResultRedirect::class, $this->getModel()->execute()); } + public function testExecuteRejectsFilterableForUnsupportedInputType() + { + $data = [ + 'frontend_input' => 'text', + 'is_filterable' => '1', + ]; + + $this->requestMock->expects($this->any()) + ->method('getParam') + ->willReturnMap([ + ['isAjax', null, null], + ['serialized_options', '[]', ''], + ['attribute_code', null, 'issue_33055_text'], + ]); + $this->formDataSerializerMock->expects($this->once())->method('unserialize')->with('')->willReturn([]); + $this->requestMock->expects($this->once())->method('getPostValue')->willReturn($data); + $this->inputTypeValidatorMock->method('isValid')->with('text')->willReturn(true); + $this->presentationMock->method('convertPresentationDataToInputType')->willReturnCallback(function ($arg) { + return $arg; + }); + $this->productHelperMock->method('getAttributeSourceModelByInputType')->with('text')->willReturn(null); + $this->productHelperMock->method('getAttributeBackendModelByInputType')->with('text')->willReturn(null); + $this->productAttributeMock->expects($this->never())->method('getDefaultValueByInput'); + $this->productAttributeMock->expects($this->never())->method('save'); + $this->messageManager->expects($this->once()) + ->method('addErrorMessage') + ->with( + $this->callback(static function ($message) { + return (string)$message === + 'Can be used only with catalog input type Yes/No, Dropdown, Multiple Select and Price.'; + }) + ); + $this->resultFactoryMock->expects($this->once()) + ->method('create') + ->with(ResultFactory::TYPE_REDIRECT) + ->willReturn($this->redirectMock); + $this->redirectMock->expects($this->once()) + ->method('setPath') + ->with('catalog/*/edit', [ + 'attribute_id' => null, + '_current' => true, + ]) + ->willReturnSelf(); + + $this->assertInstanceOf(ResultRedirect::class, $this->getModel()->execute()); + } + + public function testExecuteRejectsFilterableInSearchForUnsupportedInputType() + { + $data = [ + 'frontend_input' => 'textarea', + 'is_filterable_in_search' => '1', + ]; + + $this->requestMock->expects($this->any()) + ->method('getParam') + ->willReturnMap([ + ['isAjax', null, null], + ['serialized_options', '[]', ''], + ['attribute_code', null, 'issue_33055_textarea'], + ]); + $this->formDataSerializerMock->expects($this->once())->method('unserialize')->with('')->willReturn([]); + $this->requestMock->expects($this->once())->method('getPostValue')->willReturn($data); + $this->inputTypeValidatorMock->method('isValid')->with('textarea')->willReturn(true); + $this->presentationMock->method('convertPresentationDataToInputType')->willReturnCallback(function ($arg) { + return $arg; + }); + $this->productHelperMock->method('getAttributeSourceModelByInputType')->with('textarea')->willReturn(null); + $this->productHelperMock->method('getAttributeBackendModelByInputType')->with('textarea')->willReturn(null); + $this->productAttributeMock->expects($this->never())->method('save'); + $this->messageManager->expects($this->once())->method('addErrorMessage'); + $this->resultFactoryMock->expects($this->once()) + ->method('create') + ->with(ResultFactory::TYPE_REDIRECT) + ->willReturn($this->redirectMock); + $this->redirectMock->expects($this->any())->method('setPath')->willReturnSelf(); + + $this->assertInstanceOf(ResultRedirect::class, $this->getModel()->execute()); + } + + public function testExecuteAllowsFilterableForSupportedInputType() + { + $data = [ + 'frontend_input' => 'select', + 'is_filterable' => '1', + ]; + + $this->requestMock->expects($this->any()) + ->method('getParam') + ->willReturnMap([ + ['isAjax', null, null], + ['serialized_options', '[]', ''], + ['attribute_code', null, 'filterable_select'], + ]); + $this->formDataSerializerMock->expects($this->once())->method('unserialize')->with('')->willReturn([]); + $this->requestMock->expects($this->once())->method('getPostValue')->willReturn($data); + $this->inputTypeValidatorMock->method('isValid')->with('select')->willReturn(true); + $this->presentationMock->method('convertPresentationDataToInputType')->willReturnCallback(function ($arg) { + return $arg; + }); + $this->productHelperMock->method('getAttributeSourceModelByInputType')->with('select')->willReturn(null); + $this->productHelperMock->method('getAttributeBackendModelByInputType')->with('select')->willReturn(null); + $this->productAttributeMock->method('getDefaultValueByInput')->with('select')->willReturn(null); + $this->productAttributeMock->expects($this->once()) + ->method('addData') + ->with($this->callback(static function ($arg) { + return (int)$arg['is_filterable'] === 1; + })); + $this->messageManager->expects($this->never())->method('addErrorMessage'); + $this->resultFactoryMock->expects($this->any())->method('create')->willReturn($this->redirectMock); + $this->redirectMock->expects($this->any())->method('setPath')->willReturnSelf(); + + $this->assertInstanceOf(ResultRedirect::class, $this->getModel()->execute()); + } + private function createAttributeFactoryForGroupCollectionTest() { $attributeModel = $this->createPartialMock( @@ -1032,6 +1161,7 @@ private function createControllerWithAttributeFactory($localAttributeFactory) 'layoutFactory' => $this->layoutFactoryMock, 'formDataSerializer' => $this->formDataSerializerMock, 'presentation' => $this->presentationMock, + 'filterableAllowedInputTypes' => $this->filterableAllowedInputTypes, '_session' => $this->sessionMock ]); } diff --git a/app/code/Magento/Catalog/Test/Unit/Model/Product/Attribute/FilterableAllowedInputTypesTest.php b/app/code/Magento/Catalog/Test/Unit/Model/Product/Attribute/FilterableAllowedInputTypesTest.php new file mode 100644 index 0000000000000..02b304f068db1 --- /dev/null +++ b/app/code/Magento/Catalog/Test/Unit/Model/Product/Attribute/FilterableAllowedInputTypesTest.php @@ -0,0 +1,47 @@ +assertSame($expected, $model->isAllowed($frontendInput)); + } + + /** + * @return array + */ + public static function isAllowedDataProvider(): array + { + return [ + 'select' => ['select', true], + 'boolean' => ['boolean', true], + 'multiselect' => ['multiselect', true], + 'price' => ['price', true], + 'text' => ['text', false], + 'textarea' => ['textarea', false], + 'media_image' => ['media_image', false], + 'gallery' => ['gallery', false], + 'empty' => ['', false], + 'null' => [null, false], + ]; + } +} diff --git a/app/code/Magento/Catalog/Test/Unit/Model/Product/Attribute/RepositoryTest.php b/app/code/Magento/Catalog/Test/Unit/Model/Product/Attribute/RepositoryTest.php index edc6c31967550..bf301902cf5a1 100644 --- a/app/code/Magento/Catalog/Test/Unit/Model/Product/Attribute/RepositoryTest.php +++ b/app/code/Magento/Catalog/Test/Unit/Model/Product/Attribute/RepositoryTest.php @@ -11,6 +11,7 @@ use Magento\Catalog\Api\Data\ProductAttributeInterface; use Magento\Catalog\Api\Data\ProductInterface; use Magento\Catalog\Helper\Product; +use Magento\Catalog\Model\Product\Attribute\FilterableAllowedInputTypes; use Magento\Catalog\Model\Product\Attribute\Repository; use Magento\Catalog\Model\ResourceModel\Eav\Attribute; use Magento\Eav\Api\AttributeRepositoryInterface; @@ -133,7 +134,8 @@ protected function setUp(): void $this->eavConfigMock, $this->validatorFactoryMock, $this->searchCriteriaBuilderMock, - $this->attributeCodeValidatorMock + $this->attributeCodeValidatorMock, + new FilterableAllowedInputTypes(['boolean', 'select', 'multiselect', 'price']) ); } @@ -330,6 +332,23 @@ public static function filterableDataProvider(): array ]; } + /** + * @return void + */ + public function testSaveInputExceptionInvalidIsFilterableForMediaImage(): void + { + $this->expectException('Magento\Framework\Exception\InputException'); + $this->expectExceptionMessage('Invalid value of "1" provided for the is_filterable field.'); + $attributeMock = $this->createPartialMock( + Attribute::class, + ['getFrontendInput', 'getIsFilterable'] + ); + $attributeMock->expects($this->atLeastOnce())->method('getFrontendInput')->willReturn('media_image'); + $attributeMock->expects($this->atLeastOnce())->method('getIsFilterable')->willReturn(1); + + $this->model->save($attributeMock); + } + public function testSaveInputExceptionInvalidFieldValue() { $this->expectException('Magento\Framework\Exception\InputException'); diff --git a/app/code/Magento/Catalog/etc/di.xml b/app/code/Magento/Catalog/etc/di.xml index e5ff924a414eb..4e13dac61d6ad 100644 --- a/app/code/Magento/Catalog/etc/di.xml +++ b/app/code/Magento/Catalog/etc/di.xml @@ -77,6 +77,16 @@ + + + + boolean + select + multiselect + price + + + diff --git a/dev/tests/integration/testsuite/Magento/Catalog/Controller/Adminhtml/Product/Attribute/Save/FilterableInputTypeTest.php b/dev/tests/integration/testsuite/Magento/Catalog/Controller/Adminhtml/Product/Attribute/Save/FilterableInputTypeTest.php new file mode 100644 index 0000000000000..159da782cdfe0 --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/Catalog/Controller/Adminhtml/Product/Attribute/Save/FilterableInputTypeTest.php @@ -0,0 +1,120 @@ +createAttributeUsingDataWithErrorAndAssert($attributePostData, $errorMessage); + } + + /** + * @param array $attributePostData + * @param array $checkArray + * @return void + */ + #[DataProvider('allowedFilterableDataProvider')] + public function testCreateAttributeWithLayeredNavigationIsAllowed( + array $attributePostData, + array $checkArray + ): void { + $this->createAttributeUsingDataAndAssert($attributePostData, $checkArray); + } + + /** + * @return array + */ + public static function disallowedFilterableDataProvider(): array + { + $message = (string)__( + 'Can be used only with catalog input type Yes/No, Dropdown, Multiple Select and Price.' + ); + + $cases = []; + foreach (['text', 'textarea', 'texteditor', 'date', 'datetime', 'media_image'] as $frontendInput) { + $cases[$frontendInput . '_is_filterable'] = [ + self::postData($frontendInput, 'filterable_' . $frontendInput, '1', '0'), + $message, + ]; + } + + $cases['text_is_filterable_in_search'] = [ + self::postData('text', 'filterable_text_search', '0', '1'), + $message, + ]; + + return $cases; + } + + /** + * @return array + */ + public static function allowedFilterableDataProvider(): array + { + $cases = []; + foreach (['boolean', 'price'] as $frontendInput) { + $attributeCode = 'allowed_filterable_' . $frontendInput; + $cases[$frontendInput . '_is_filterable'] = [ + self::postData($frontendInput, $attributeCode, '1', '0'), + [ + 'attribute_code' => $attributeCode, + 'frontend_input' => $frontendInput, + 'is_filterable' => 1, + ], + ]; + } + + return $cases; + } + + /** + * @param string $frontendInput + * @param string $attributeCode + * @param string $isFilterable + * @param string $isFilterableInSearch + * @return array + */ + private static function postData( + string $frontendInput, + string $attributeCode, + string $isFilterable, + string $isFilterableInSearch + ): array { + return [ + 'frontend_label' => [ + Store::DEFAULT_STORE_ID => 'Test attribute name', + ], + 'frontend_input' => $frontendInput, + 'is_required' => '0', + 'attribute_code' => $attributeCode, + 'is_global' => '1', + 'is_unique' => '0', + 'is_searchable' => $isFilterableInSearch === '1' ? '1' : '0', + 'is_filterable' => $isFilterable, + 'is_filterable_in_search' => $isFilterableInSearch, + ]; + } +} diff --git a/dev/tests/integration/testsuite/Magento/Catalog/Model/Product/Attribute/RepositoryTest.php b/dev/tests/integration/testsuite/Magento/Catalog/Model/Product/Attribute/RepositoryTest.php index 6a83f629a1b29..1c893ea448a74 100644 --- a/dev/tests/integration/testsuite/Magento/Catalog/Model/Product/Attribute/RepositoryTest.php +++ b/dev/tests/integration/testsuite/Magento/Catalog/Model/Product/Attribute/RepositoryTest.php @@ -12,6 +12,7 @@ use Magento\Catalog\Api\ProductAttributeRepositoryInterface; use Magento\Catalog\Setup\CategorySetup; use Magento\Eav\Model\Entity\Attribute\ScopedAttributeInterface; +use Magento\Eav\Model\Validator\Attribute\Code; use Magento\Framework\Exception\InputException; use Magento\Framework\ObjectManagerInterface; use Magento\TestFramework\Helper\Bootstrap; @@ -49,6 +50,17 @@ protected function setUp(): void $this->objectManager = Bootstrap::getObjectManager(); $this->repository = $this->objectManager->get(ProductAttributeRepositoryInterface::class); $this->attributeFactory = $this->objectManager->get(ProductAttributeInterfaceFactory::class); + $this->clearAttributeCodeValidatorMessages(); + } + + /** + * Shared attribute-code validator keeps messages between calls. + */ + private function clearAttributeCodeValidatorMessages(): void + { + $validator = $this->objectManager->get(Code::class); + $method = new \ReflectionMethod($validator, '_clearMessages'); + $method->invoke($validator); } /** @@ -112,6 +124,88 @@ public static function errorProvider(): array ]; } + /** + * @param string $frontendInput + * @param string $field + * @param int $value + * @return void + */ + #[DataProvider('invalidFilterableInputTypeProvider')] + public function testSaveRejectsFilterableForUnsupportedInputType( + string $frontendInput, + string $field, + int $value + ): void { + $this->expectExceptionObject(InputException::invalidFieldValue($field, $value)); + $this->createdAttribute = $this->saveAttributeWithData( + $this->hydrateData( + [ + 'attribute_code' => 'rej_' . $frontendInput . '_' . $field, + 'frontend_input' => $frontendInput, + 'frontend_label' => 'Rejected ' . $frontendInput, + $field => $value, + ] + ) + ); + } + + /** + * @return array + */ + public static function invalidFilterableInputTypeProvider(): array + { + return [ + 'text_is_filterable' => ['text', ProductAttributeInterface::IS_FILTERABLE, 1], + 'textarea_is_filterable' => ['textarea', ProductAttributeInterface::IS_FILTERABLE, 1], + 'date_is_filterable' => ['date', ProductAttributeInterface::IS_FILTERABLE, 1], + 'datetime_is_filterable' => ['datetime', ProductAttributeInterface::IS_FILTERABLE, 1], + 'media_image_is_filterable' => ['media_image', ProductAttributeInterface::IS_FILTERABLE, 1], + 'text_is_filterable_in_search' => ['text', ProductAttributeInterface::IS_FILTERABLE_IN_SEARCH, 1], + ]; + } + + /** + * @return void + */ + public function testSaveAllowsFilterableForSupportedInputType(): void + { + $this->createdAttribute = $this->saveAttributeWithData( + $this->hydrateData( + [ + 'attribute_code' => 'repo_filt_bool', + 'frontend_input' => 'boolean', + 'frontend_label' => 'Allowed Filterable Boolean', + ProductAttributeInterface::IS_FILTERABLE => 1, + ] + ) + ); + + $this->assertSame(1, (int)$this->createdAttribute->getIsFilterable()); + } + + /** + * @return void + */ + public function testSaveRejectsEnablingFilterableOnExistingTextAttribute(): void + { + $this->createdAttribute = $this->saveAttributeWithData( + $this->hydrateData( + [ + 'attribute_code' => 'repo_txt_then_filt', + 'frontend_input' => 'text', + 'frontend_label' => 'Text Then Filterable', + ProductAttributeInterface::IS_FILTERABLE => 0, + ] + ) + ); + $this->createdAttribute->setIsFilterable(1); + + $this->expectExceptionObject( + InputException::invalidFieldValue(ProductAttributeInterface::IS_FILTERABLE, 1) + ); + $this->repository->save($this->createdAttribute); + } + /** * Save product attribute with data *