From a9530b0c5c5875ed67c44d618253f437737b5938 Mon Sep 17 00:00:00 2001 From: Pierre Boissinot Date: Fri, 9 Mar 2018 10:29:08 +0100 Subject: [PATCH 1/5] phone number format as option instead of hard set to NATIONAL (behavior like single_text widget) --- .../DataTransformer/PhoneNumberToArrayTransformer.php | 11 +++++++++-- Form/Type/PhoneNumberType.php | 2 +- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/Form/DataTransformer/PhoneNumberToArrayTransformer.php b/Form/DataTransformer/PhoneNumberToArrayTransformer.php index 81b2cc7b..6b1e191d 100644 --- a/Form/DataTransformer/PhoneNumberToArrayTransformer.php +++ b/Form/DataTransformer/PhoneNumberToArrayTransformer.php @@ -28,14 +28,21 @@ class PhoneNumberToArrayTransformer implements DataTransformerInterface */ private $countryChoices; + /** + * @var int + */ + private $format; + /** * Constructor. * * @param array $countryChoices + * @param int $format */ - public function __construct(array $countryChoices) + public function __construct(array $countryChoices, $format = PhoneNumberFormat::INTERNATIONAL) { $this->countryChoices = $countryChoices; + $this->format = $format; } /** @@ -57,7 +64,7 @@ public function transform($phoneNumber) return array( 'country' => $util->getRegionCodeForNumber($phoneNumber), - 'number' => $util->format($phoneNumber, PhoneNumberFormat::NATIONAL), + 'number' => $util->format($phoneNumber, $this->format), ); } diff --git a/Form/Type/PhoneNumberType.php b/Form/Type/PhoneNumberType.php index 2f01548c..8574b4fc 100644 --- a/Form/Type/PhoneNumberType.php +++ b/Form/Type/PhoneNumberType.php @@ -104,7 +104,7 @@ public function buildForm(FormBuilderInterface $builder, array $options) $builder ->add('country', $choiceType, $countryOptions) ->add('number', $textType, $numberOptions) - ->addViewTransformer(new PhoneNumberToArrayTransformer($transformerChoices)); + ->addViewTransformer(new PhoneNumberToArrayTransformer($transformerChoices, $options['format'])); } else { $builder->addViewTransformer( new PhoneNumberToStringTransformer($options['default_region'], $options['format']) From d9f892ab1ca50d284d51532e4804506a6dd32fa5 Mon Sep 17 00:00:00 2001 From: Pierre Boissinot Date: Fri, 9 Mar 2018 14:50:15 +0100 Subject: [PATCH 2/5] update test cases --- .../PhoneNumberToArrayTransformer.php | 5 ++++- .../PhoneNumberToArrayTransformerTest.php | 15 +++++++++++---- Tests/Form/Type/PhoneNumberTypeTest.php | 8 ++++---- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/Form/DataTransformer/PhoneNumberToArrayTransformer.php b/Form/DataTransformer/PhoneNumberToArrayTransformer.php index 6b1e191d..c949ee4e 100644 --- a/Form/DataTransformer/PhoneNumberToArrayTransformer.php +++ b/Form/DataTransformer/PhoneNumberToArrayTransformer.php @@ -39,7 +39,10 @@ class PhoneNumberToArrayTransformer implements DataTransformerInterface * @param array $countryChoices * @param int $format */ - public function __construct(array $countryChoices, $format = PhoneNumberFormat::INTERNATIONAL) + public function __construct( + array $countryChoices, + $format = PhoneNumberFormat::INTERNATIONAL + ) { $this->countryChoices = $countryChoices; $this->format = $format; diff --git a/Tests/Form/DataTransformer/PhoneNumberToArrayTransformerTest.php b/Tests/Form/DataTransformer/PhoneNumberToArrayTransformerTest.php index 90d1681a..a3823af1 100644 --- a/Tests/Form/DataTransformer/PhoneNumberToArrayTransformerTest.php +++ b/Tests/Form/DataTransformer/PhoneNumberToArrayTransformerTest.php @@ -46,9 +46,9 @@ public function testConstructor() /** * @dataProvider transformProvider */ - public function testTransform(array $countryChoices, $actual, $expected) + public function testTransform(array $countryChoices, $format, $actual, $expected) { - $transformer = new PhoneNumberToArrayTransformer($countryChoices); + $transformer = new PhoneNumberToArrayTransformer($countryChoices, $format); if (is_array($actual)) { try { @@ -71,8 +71,9 @@ public function testTransform(array $countryChoices, $actual, $expected) /** * 0 => Country choices - * 1 => Actual value - * 2 => Expected result + * 1 => Format + * 2 => Actual value + * 3 => Expected result */ public function transformProvider() { @@ -80,30 +81,36 @@ public function transformProvider() array( array('GB'), null, + null, array('country' => '', 'number' => ''), ), array( array('GB'), + PhoneNumberFormat::NATIONAL, array('country' => 'GB', 'number' => '01234567890'), array('country' => 'GB', 'number' => '01234 567890'), ), array(// Wrong country code, but matching country exists. array('GB', 'JE'), + PhoneNumberFormat::NATIONAL, array('country' => 'JE', 'number' => '01234567890'), array('country' => 'GB', 'number' => '01234 567890'), ), array(// Wrong country code, but matching country exists. array('GB', 'JE'), + PhoneNumberFormat::NATIONAL, array('country' => 'JE', 'number' => '+441234567890'), array('country' => 'GB', 'number' => '01234 567890'), ), array(// Country code not in list. array('US'), + null, array('country' => 'GB', 'number' => '01234567890'), self::TRANSFORMATION_FAILED, ), array( array('US'), + null, array('country' => 'GB', 'number' => 'foo'), self::TRANSFORMATION_FAILED, ), diff --git a/Tests/Form/Type/PhoneNumberTypeTest.php b/Tests/Form/Type/PhoneNumberTypeTest.php index 7a4394d3..a5a94023 100644 --- a/Tests/Form/Type/PhoneNumberTypeTest.php +++ b/Tests/Form/Type/PhoneNumberTypeTest.php @@ -109,10 +109,10 @@ public function testCountryChoiceValues($input, $options, $output) public function countryChoiceValuesProvider() { return array( - array(array('country' => 'GB', 'number' => '01234 567890'), array(), array('country' => 'GB', 'number' => '01234 567890')), - array(array('country' => 'GB', 'number' => '+44 1234 567890'), array(), array('country' => 'GB', 'number' => '01234 567890')), - array(array('country' => 'GB', 'number' => '1234 567890'), array(), array('country' => 'GB', 'number' => '01234 567890')), - array(array('country' => 'GB', 'number' => '+1 650-253-0000'), array(), array('country' => 'US', 'number' => '(650) 253-0000')), + array(array('country' => 'GB', 'number' => '01234 567890'), array('format' => PhoneNumberFormat::NATIONAL), array('country' => 'GB', 'number' => '01234 567890')), + array(array('country' => 'GB', 'number' => '+44 1234 567890'), array('format' => PhoneNumberFormat::NATIONAL), array('country' => 'GB', 'number' => '01234 567890')), + array(array('country' => 'GB', 'number' => '1234 567890'), array('format' => PhoneNumberFormat::NATIONAL), array('country' => 'GB', 'number' => '01234 567890')), + array(array('country' => 'GB', 'number' => '+1 650-253-0000'), array('format' => PhoneNumberFormat::NATIONAL), array('country' => 'US', 'number' => '(650) 253-0000')), array(array('country' => '', 'number' => ''), array(), array('country' => '', 'number' => '')), ); } From 1b520be7aa7681d858c21be4143b7f0bdafaa53a Mon Sep 17 00:00:00 2001 From: Pierre Boissinot Date: Fri, 9 Mar 2018 15:46:35 +0100 Subject: [PATCH 3/5] set default format national to not introduce a breaking change --- Form/DataTransformer/PhoneNumberToArrayTransformer.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Form/DataTransformer/PhoneNumberToArrayTransformer.php b/Form/DataTransformer/PhoneNumberToArrayTransformer.php index c949ee4e..73c69cad 100644 --- a/Form/DataTransformer/PhoneNumberToArrayTransformer.php +++ b/Form/DataTransformer/PhoneNumberToArrayTransformer.php @@ -41,7 +41,7 @@ class PhoneNumberToArrayTransformer implements DataTransformerInterface */ public function __construct( array $countryChoices, - $format = PhoneNumberFormat::INTERNATIONAL + $format = PhoneNumberFormat::NATIONAL ) { $this->countryChoices = $countryChoices; From 8cd7a42e6512f07ee0f96d4426b93a17e1b2d8d2 Mon Sep 17 00:00:00 2001 From: Pierre Boissinot Date: Tue, 13 Mar 2018 14:21:47 +0100 Subject: [PATCH 4/5] the default format option depends on the widget chosen (non regression) --- Form/Type/PhoneNumberType.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Form/Type/PhoneNumberType.php b/Form/Type/PhoneNumberType.php index 8574b4fc..9ef2ac72 100644 --- a/Form/Type/PhoneNumberType.php +++ b/Form/Type/PhoneNumberType.php @@ -144,7 +144,11 @@ public function configureOptions(OptionsResolver $resolver) return PhoneNumberType::WIDGET_SINGLE_TEXT !== $options['widget']; }, 'default_region' => PhoneNumberUtil::UNKNOWN_REGION, - 'format' => PhoneNumberFormat::INTERNATIONAL, + 'format' => function (Options $options) { + return PhoneNumberType::WIDGET_SINGLE_TEXT === $options['widget'] + ? PhoneNumberFormat::INTERNATIONAL + : PhoneNumberFormat::NATIONAL; + }, 'invalid_message' => 'This value is not a valid phone number.', 'by_reference' => false, 'error_bubbling' => false, From d403883b4aea13dd6eae9e73e6e2dfc68e4c6d35 Mon Sep 17 00:00:00 2001 From: Pierre Boissinot Date: Tue, 13 Mar 2018 14:22:21 +0100 Subject: [PATCH 5/5] add some tests for default number format option and recent changes --- Tests/Form/Type/PhoneNumberTypeTest.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Tests/Form/Type/PhoneNumberTypeTest.php b/Tests/Form/Type/PhoneNumberTypeTest.php index a5a94023..bd4681c8 100644 --- a/Tests/Form/Type/PhoneNumberTypeTest.php +++ b/Tests/Form/Type/PhoneNumberTypeTest.php @@ -109,10 +109,14 @@ public function testCountryChoiceValues($input, $options, $output) public function countryChoiceValuesProvider() { return array( + array(array('country' => 'GB', 'number' => '01234 567890'), array(), array('country' => 'GB', 'number' => '01234 567890')), array(array('country' => 'GB', 'number' => '01234 567890'), array('format' => PhoneNumberFormat::NATIONAL), array('country' => 'GB', 'number' => '01234 567890')), + array(array('country' => 'GB', 'number' => '01234 567890'), array('format' => PhoneNumberFormat::INTERNATIONAL), array('country' => 'GB', 'number' => '+44 1234 567890')), + array(array('country' => 'GB', 'number' => '+44 1234 567890'), array(), array('country' => 'GB', 'number' => '01234 567890')), + array(array('country' => 'GB', 'number' => '+44 1234 567890'), array('format' => PhoneNumberFormat::INTERNATIONAL), array('country' => 'GB', 'number' => '+44 1234 567890')), array(array('country' => 'GB', 'number' => '+44 1234 567890'), array('format' => PhoneNumberFormat::NATIONAL), array('country' => 'GB', 'number' => '01234 567890')), - array(array('country' => 'GB', 'number' => '1234 567890'), array('format' => PhoneNumberFormat::NATIONAL), array('country' => 'GB', 'number' => '01234 567890')), - array(array('country' => 'GB', 'number' => '+1 650-253-0000'), array('format' => PhoneNumberFormat::NATIONAL), array('country' => 'US', 'number' => '(650) 253-0000')), + array(array('country' => 'GB', 'number' => '1234 567890'), array(), array('country' => 'GB', 'number' => '01234 567890')), + array(array('country' => 'GB', 'number' => '+1 650-253-0000'), array(), array('country' => 'US', 'number' => '(650) 253-0000')), array(array('country' => '', 'number' => ''), array(), array('country' => '', 'number' => '')), ); }