From 7aa9a9c35990d4f7115661a03a1cce39733a21c9 Mon Sep 17 00:00:00 2001 From: "vadim.malesh" Date: Fri, 15 May 2020 11:05:02 +0300 Subject: [PATCH 1/3] fix --- app/code/Magento/Customer/Model/Options.php | 6 +- .../Magento/Customer/Model/OptionsTest.php | 165 ++++++++++++++++++ 2 files changed, 169 insertions(+), 2 deletions(-) create mode 100644 dev/tests/integration/testsuite/Magento/Customer/Model/OptionsTest.php diff --git a/app/code/Magento/Customer/Model/Options.php b/app/code/Magento/Customer/Model/Options.php index 71e70f8e14208..bc093502fb4fc 100644 --- a/app/code/Magento/Customer/Model/Options.php +++ b/app/code/Magento/Customer/Model/Options.php @@ -97,12 +97,14 @@ private function prepareNamePrefixSuffixOptions($options, $isOptional = false) if (empty($options)) { return false; } + $result = []; - $options = array_filter(explode(';', $options)); + $options = explode(';', $options); foreach ($options as $value) { - $value = $this->escaper->escapeHtml(trim($value)); + $value = $this->escaper->escapeHtml(trim($value)) ?: ' '; $result[$value] = $value; } + if ($isOptional && trim(current($options))) { $result = array_merge([' ' => ' '], $result); } diff --git a/dev/tests/integration/testsuite/Magento/Customer/Model/OptionsTest.php b/dev/tests/integration/testsuite/Magento/Customer/Model/OptionsTest.php new file mode 100644 index 0000000000000..f77852f230b65 --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/Customer/Model/OptionsTest.php @@ -0,0 +1,165 @@ + ' ', 'v1' => 'v1', 'v2' => 'v2']; + private const STUB_EXPECTED_VALUES = ['v1' => 'v1', 'v2' => 'v2']; + + /** + * @var Options + */ + private $model; + + /** + * @var ObjectManagerInterface + */ + private $objectManager; + + /** + * @inheritdoc + */ + protected function setUp(): void + { + $this->objectManager = Bootstrap::getObjectManager(); + $this->model = $this->objectManager->create(Options::class); + } + + /** + * Test suffix and prefix options + * + * @dataProvider optionsDataProvider + * + * @param string $optionType + * @param array $showOptionConfig + * @param array $optionValuesConfig + * @param array $expectedOptions + * @param int $expectedCount + * @return void + */ + public function testOptions( + string $optionType, + array $showOptionConfig, + array $optionValuesConfig, + array $expectedOptions, + int $expectedCount + ): void { + $this->setConfig($showOptionConfig); + $this->setConfig($optionValuesConfig); + + /** @var array $options */ + $options = $optionType === self::STUB_OPTION_PREFIX_NAME + ? $this->model->getNamePrefixOptions() + : $this->model->getNameSuffixOptions(); + + $this->assertCount($expectedCount, $options); + $this->assertEquals($expectedOptions, $options); + } + + /** + * Set config param + * + * @param array $data + * @param string|null $scopeType + * @param string|null $scopeCode + * @return void + */ + private function setConfig( + array $data, + ?string $scopeType = ScopeInterface::SCOPE_STORE, + ?string $scopeCode = 'default' + ): void { + $path = array_key_first($data); + $this->objectManager->get(MutableScopeConfigInterface::class) + ->setValue($path, $data[$path], $scopeType, $scopeCode); + } + + /** + * DataProvider for testOptions() + * + * @return array + */ + public function optionsDataProvider(): array + { + return [ + 'prefix_required_with_blank_option' => [ + self::STUB_OPTION_PREFIX_NAME, + [self::XML_PATH_PREFIX_SHOW => Nooptreq::VALUE_REQUIRED], + [self::XML_PATH_PREFIX_OPTIONS => self::STUB_CONFIG_VALUES_WITH_BLANK_OPTION], + self::STUB_EXPECTED_VALUES_WITH_BLANK_OPTION, + 3, + ], + 'prefix_required' => [ + self::STUB_OPTION_PREFIX_NAME, + [self::XML_PATH_PREFIX_SHOW => Nooptreq::VALUE_REQUIRED], + [self::XML_PATH_PREFIX_OPTIONS => self::STUB_CONFIG_VALUES], + self::STUB_EXPECTED_VALUES, + 2, + ], + 'prefix_optional' => [ + self::STUB_OPTION_PREFIX_NAME, + [self::XML_PATH_PREFIX_SHOW => Nooptreq::VALUE_OPTIONAL], + [self::XML_PATH_PREFIX_OPTIONS => self::STUB_CONFIG_VALUES], + self::STUB_EXPECTED_VALUES_WITH_BLANK_OPTION, + 3, + ], + 'suffix_optional' => [ + self::STUB_OPTION_SUFFIX_NAME, + [self::XML_PATH_SUFFIX_SHOW => Nooptreq::VALUE_OPTIONAL], + [self::XML_PATH_SUFFIX_OPTIONS => self::STUB_CONFIG_VALUES], + self::STUB_EXPECTED_VALUES_WITH_BLANK_OPTION, + 3, + ], + 'suffix_optional_with_blank_option' => [ + self::STUB_OPTION_SUFFIX_NAME, + [self::XML_PATH_SUFFIX_SHOW => Nooptreq::VALUE_OPTIONAL], + [self::XML_PATH_SUFFIX_OPTIONS => self::STUB_CONFIG_VALUES_WITH_BLANK_OPTION], + self::STUB_EXPECTED_VALUES_WITH_BLANK_OPTION, + 3, + ], + 'suffix_required_with_blank_option' => [ + self::STUB_OPTION_SUFFIX_NAME, + [self::XML_PATH_SUFFIX_SHOW => Nooptreq::VALUE_OPTIONAL], + [self::XML_PATH_SUFFIX_OPTIONS => self::STUB_CONFIG_VALUES_WITH_BLANK_OPTION], + self::STUB_EXPECTED_VALUES_WITH_BLANK_OPTION, + 3, + ], + ]; + } + + /** + * @inheritdoc + */ + protected function tearDown(): void + { + $this->objectManager->removeSharedInstance(Address::class); + } +} From daecbb410a7481d8ea0e80e8c57479e02beb93b4 Mon Sep 17 00:00:00 2001 From: "vadim.malesh" Date: Mon, 18 May 2020 17:06:28 +0300 Subject: [PATCH 2/3] improve test --- .../Magento/Customer/Model/OptionsTest.php | 63 ++++++++----------- 1 file changed, 27 insertions(+), 36 deletions(-) diff --git a/dev/tests/integration/testsuite/Magento/Customer/Model/OptionsTest.php b/dev/tests/integration/testsuite/Magento/Customer/Model/OptionsTest.php index f77852f230b65..5deda0803fff0 100644 --- a/dev/tests/integration/testsuite/Magento/Customer/Model/OptionsTest.php +++ b/dev/tests/integration/testsuite/Magento/Customer/Model/OptionsTest.php @@ -27,13 +27,6 @@ class OptionsTest extends TestCase private const XML_PATH_PREFIX_SHOW = 'customer/address/prefix_show'; private const XML_PATH_PREFIX_OPTIONS = 'customer/address/prefix_options'; - private const STUB_OPTION_PREFIX_NAME = 'prefix'; - private const STUB_OPTION_SUFFIX_NAME = 'suffix'; - private const STUB_CONFIG_VALUES = 'v1;v2'; - private const STUB_CONFIG_VALUES_WITH_BLANK_OPTION = ';v1;v2'; - private const STUB_EXPECTED_VALUES_WITH_BLANK_OPTION = [' ' => ' ', 'v1' => 'v1', 'v2' => 'v2']; - private const STUB_EXPECTED_VALUES = ['v1' => 'v1', 'v2' => 'v2']; - /** * @var Options */ @@ -62,25 +55,22 @@ protected function setUp(): void * @param array $showOptionConfig * @param array $optionValuesConfig * @param array $expectedOptions - * @param int $expectedCount * @return void */ public function testOptions( string $optionType, array $showOptionConfig, array $optionValuesConfig, - array $expectedOptions, - int $expectedCount + array $expectedOptions ): void { $this->setConfig($showOptionConfig); $this->setConfig($optionValuesConfig); /** @var array $options */ - $options = $optionType === self::STUB_OPTION_PREFIX_NAME + $options = $optionType === 'prefix' ? $this->model->getNamePrefixOptions() : $this->model->getNameSuffixOptions(); - $this->assertCount($expectedCount, $options); $this->assertEquals($expectedOptions, $options); } @@ -109,48 +99,49 @@ private function setConfig( */ public function optionsDataProvider(): array { + $optionPrefixName = 'prefix'; + $optionSuffixName = 'suffix'; + $optionValues = 'v1;v2'; + $optionValuesWithBlank = ';v1;v2'; + $expectedValuesWithBlank = [' ' => ' ', 'v1' => 'v1', 'v2' => 'v2']; + $expectedValues = ['v1' => 'v1', 'v2' => 'v2']; + return [ 'prefix_required_with_blank_option' => [ - self::STUB_OPTION_PREFIX_NAME, + $optionPrefixName, [self::XML_PATH_PREFIX_SHOW => Nooptreq::VALUE_REQUIRED], - [self::XML_PATH_PREFIX_OPTIONS => self::STUB_CONFIG_VALUES_WITH_BLANK_OPTION], - self::STUB_EXPECTED_VALUES_WITH_BLANK_OPTION, - 3, + [self::XML_PATH_PREFIX_OPTIONS => $optionValuesWithBlank], + $expectedValuesWithBlank, ], 'prefix_required' => [ - self::STUB_OPTION_PREFIX_NAME, + $optionPrefixName, [self::XML_PATH_PREFIX_SHOW => Nooptreq::VALUE_REQUIRED], - [self::XML_PATH_PREFIX_OPTIONS => self::STUB_CONFIG_VALUES], - self::STUB_EXPECTED_VALUES, - 2, + [self::XML_PATH_PREFIX_OPTIONS => $optionValues], + $expectedValues, ], 'prefix_optional' => [ - self::STUB_OPTION_PREFIX_NAME, + $optionPrefixName, [self::XML_PATH_PREFIX_SHOW => Nooptreq::VALUE_OPTIONAL], - [self::XML_PATH_PREFIX_OPTIONS => self::STUB_CONFIG_VALUES], - self::STUB_EXPECTED_VALUES_WITH_BLANK_OPTION, - 3, + [self::XML_PATH_PREFIX_OPTIONS => $optionValues], + $expectedValuesWithBlank, ], 'suffix_optional' => [ - self::STUB_OPTION_SUFFIX_NAME, + $optionSuffixName, [self::XML_PATH_SUFFIX_SHOW => Nooptreq::VALUE_OPTIONAL], - [self::XML_PATH_SUFFIX_OPTIONS => self::STUB_CONFIG_VALUES], - self::STUB_EXPECTED_VALUES_WITH_BLANK_OPTION, - 3, + [self::XML_PATH_SUFFIX_OPTIONS => $optionValues], + $expectedValuesWithBlank, ], 'suffix_optional_with_blank_option' => [ - self::STUB_OPTION_SUFFIX_NAME, + $optionSuffixName, [self::XML_PATH_SUFFIX_SHOW => Nooptreq::VALUE_OPTIONAL], - [self::XML_PATH_SUFFIX_OPTIONS => self::STUB_CONFIG_VALUES_WITH_BLANK_OPTION], - self::STUB_EXPECTED_VALUES_WITH_BLANK_OPTION, - 3, + [self::XML_PATH_SUFFIX_OPTIONS => $optionValuesWithBlank], + $expectedValuesWithBlank, ], 'suffix_required_with_blank_option' => [ - self::STUB_OPTION_SUFFIX_NAME, + $optionSuffixName, [self::XML_PATH_SUFFIX_SHOW => Nooptreq::VALUE_OPTIONAL], - [self::XML_PATH_SUFFIX_OPTIONS => self::STUB_CONFIG_VALUES_WITH_BLANK_OPTION], - self::STUB_EXPECTED_VALUES_WITH_BLANK_OPTION, - 3, + [self::XML_PATH_SUFFIX_OPTIONS => $optionValuesWithBlank], + $expectedValuesWithBlank, ], ]; } From e5be53f628f6170324572132bf66742b6fb63115 Mon Sep 17 00:00:00 2001 From: "vadim.malesh" Date: Tue, 21 Jul 2020 15:19:41 +0300 Subject: [PATCH 3/3] remove filtering by unique value --- app/code/Magento/Customer/Model/Options.php | 5 ++--- .../testsuite/Magento/Customer/Model/OptionsTest.php | 12 ++++++++++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/app/code/Magento/Customer/Model/Options.php b/app/code/Magento/Customer/Model/Options.php index bc093502fb4fc..7496c2c0bc0bf 100644 --- a/app/code/Magento/Customer/Model/Options.php +++ b/app/code/Magento/Customer/Model/Options.php @@ -101,12 +101,11 @@ private function prepareNamePrefixSuffixOptions($options, $isOptional = false) $result = []; $options = explode(';', $options); foreach ($options as $value) { - $value = $this->escaper->escapeHtml(trim($value)) ?: ' '; - $result[$value] = $value; + $result[] = $this->escaper->escapeHtml(trim($value)) ?: ' '; } if ($isOptional && trim(current($options))) { - $result = array_merge([' ' => ' '], $result); + $result = array_merge([' '], $result); } return $result; diff --git a/dev/tests/integration/testsuite/Magento/Customer/Model/OptionsTest.php b/dev/tests/integration/testsuite/Magento/Customer/Model/OptionsTest.php index 5deda0803fff0..879707edd9224 100644 --- a/dev/tests/integration/testsuite/Magento/Customer/Model/OptionsTest.php +++ b/dev/tests/integration/testsuite/Magento/Customer/Model/OptionsTest.php @@ -102,9 +102,11 @@ public function optionsDataProvider(): array $optionPrefixName = 'prefix'; $optionSuffixName = 'suffix'; $optionValues = 'v1;v2'; + $expectedValues = ['v1', 'v2']; $optionValuesWithBlank = ';v1;v2'; - $expectedValuesWithBlank = [' ' => ' ', 'v1' => 'v1', 'v2' => 'v2']; - $expectedValues = ['v1' => 'v1', 'v2' => 'v2']; + $expectedValuesWithBlank = [' ', 'v1', 'v2']; + $optionValuesWithTwoBlank = ';v1;v2;'; + $expectedValuesTwoBlank = [' ', 'v1', 'v2', ' ']; return [ 'prefix_required_with_blank_option' => [ @@ -119,6 +121,12 @@ public function optionsDataProvider(): array [self::XML_PATH_PREFIX_OPTIONS => $optionValues], $expectedValues, ], + 'prefix_required_with_two_blank_option' => [ + $optionPrefixName, + [self::XML_PATH_PREFIX_SHOW => Nooptreq::VALUE_REQUIRED], + [self::XML_PATH_PREFIX_OPTIONS => $optionValuesWithTwoBlank], + $expectedValuesTwoBlank, + ], 'prefix_optional' => [ $optionPrefixName, [self::XML_PATH_PREFIX_SHOW => Nooptreq::VALUE_OPTIONAL],