OP-264 - Fix for validation, added support for Polish prefixes, remov…

…ed min/max phone validation message
SzymonKostrubiec · Jun 3, 2024 · 31b65ce · 31b65ce
1 parent 0b7662e
commit 31b65ce
Show file tree

Hide file tree

Showing 4 changed files with 215 additions and 20 deletions.
diff --git a/spec/Validator/HasValidPhoneNumberInPostOrderValidatorSpec.php b/spec/Validator/HasValidPhoneNumberInPostOrderValidatorSpec.php
@@ -89,7 +89,7 @@ public function it_should_do_nothing_if_phone_number_is_not_set(
         $this->validate($value, $constraint);
     }
 
-    public function it_should_add_violation_if_phone_number_is_too_short(
+    public function it_should_add_violation_if_phone_number_is_too_short_without_polish_prefix(
         ExecutionContextInterface $context,
         Constraint $constraint,
         OrderInterface $value,
@@ -101,9 +101,53 @@ public function it_should_add_violation_if_phone_number_is_too_short(
         $value->getShippingAddress()->willReturn($addressBuilder);
         $shippingMethodChecker->isInPost(Argument::type(OrderInterface::class))->willReturn(true);
 
-        $context->buildViolation(HasValidPhoneNumberInPostOrder::PHONE_NUMBER_IS_TOO_SHORT_MESSAGE)
+        $context->buildViolation(HasValidPhoneNumberInPostOrder::PHONE_NUMBER_LENGTH_INCORRECT)
             ->willReturn($violationBuilder);
-        $violationBuilder->setParameter('{{ limit }}', (string) HasValidPhoneNumberInPostOrder::POLISH_PHONE_NUMBER_DEFAULT_LENGTH)
+        $violationBuilder->atPath('shippingAddress.phoneNumber')
+            ->willReturn($violationBuilder);
+
+        $violationBuilder->addViolation()->shouldBeCalled();
+
+        $this->initialize($context);
+        $this->validate($value, $constraint);
+    }
+
+    public function it_should_add_violation_if_phone_number_is_too_short_with_polish_prefix(
+        ExecutionContextInterface $context,
+        Constraint $constraint,
+        OrderInterface $value,
+        ConstraintViolationBuilderInterface $violationBuilder,
+        ShippingMethodCheckerInterface $shippingMethodChecker,
+    ): void {
+        $addressBuilder = AddressBuilder::create()->withPhoneNumber('+48.123')->build();
+        $value->implement(InPostPointsAwareInterface::class);
+        $value->getShippingAddress()->willReturn($addressBuilder);
+        $shippingMethodChecker->isInPost(Argument::type(OrderInterface::class))->willReturn(true);
+
+        $context->buildViolation(HasValidPhoneNumberInPostOrder::PHONE_NUMBER_LENGTH_INCORRECT)
+            ->willReturn($violationBuilder);
+        $violationBuilder->atPath('shippingAddress.phoneNumber')
+            ->willReturn($violationBuilder);
+
+        $violationBuilder->addViolation()->shouldBeCalled();
+
+        $this->initialize($context);
+        $this->validate($value, $constraint);
+    }
+
+    public function it_should_add_violation_if_phone_number_is_too_short_with_polish_long_prefix(
+        ExecutionContextInterface $context,
+        Constraint $constraint,
+        OrderInterface $value,
+        ConstraintViolationBuilderInterface $violationBuilder,
+        ShippingMethodCheckerInterface $shippingMethodChecker,
+    ): void {
+        $addressBuilder = AddressBuilder::create()->withPhoneNumber('+0048 123')->build();
+        $value->implement(InPostPointsAwareInterface::class);
+        $value->getShippingAddress()->willReturn($addressBuilder);
+        $shippingMethodChecker->isInPost(Argument::type(OrderInterface::class))->willReturn(true);
+
+        $context->buildViolation(HasValidPhoneNumberInPostOrder::PHONE_NUMBER_LENGTH_INCORRECT)
             ->willReturn($violationBuilder);
         $violationBuilder->atPath('shippingAddress.phoneNumber')
             ->willReturn($violationBuilder);
@@ -114,7 +158,7 @@ public function it_should_add_violation_if_phone_number_is_too_short(
         $this->validate($value, $constraint);
     }
 
-    public function it_should_add_violation_if_phone_number_is_too_long(
+    public function it_should_add_violation_if_phone_number_is_too_long_without_polish_prefix(
         ExecutionContextInterface $context,
         Constraint $constraint,
         OrderInterface $value,
@@ -126,9 +170,53 @@ public function it_should_add_violation_if_phone_number_is_too_long(
         $value->getShippingAddress()->willReturn($addressBuilder);
         $shippingMethodChecker->isInPost(Argument::type(OrderInterface::class))->willReturn(true);
 
-        $context->buildViolation(HasValidPhoneNumberInPostOrder::PHONE_NUMBER_IS_TOO_LONG_MESSAGE)
+        $context->buildViolation(HasValidPhoneNumberInPostOrder::PHONE_NUMBER_LENGTH_INCORRECT)
+            ->willReturn($violationBuilder);
+        $violationBuilder->atPath('shippingAddress.phoneNumber')
+            ->willReturn($violationBuilder);
+
+        $violationBuilder->addViolation()->shouldBeCalled();
+
+        $this->initialize($context);
+        $this->validate($value, $constraint);
+    }
+
+    public function it_should_add_violation_if_phone_number_is_too_long_with_polish_prefix(
+        ExecutionContextInterface $context,
+        Constraint $constraint,
+        OrderInterface $value,
+        ConstraintViolationBuilderInterface $violationBuilder,
+        ShippingMethodCheckerInterface $shippingMethodChecker,
+    ): void {
+        $addressBuilder = AddressBuilder::create()->withPhoneNumber('+48 1234567890')->build();
+        $value->implement(InPostPointsAwareInterface::class);
+        $value->getShippingAddress()->willReturn($addressBuilder);
+        $shippingMethodChecker->isInPost(Argument::type(OrderInterface::class))->willReturn(true);
+
+        $context->buildViolation(HasValidPhoneNumberInPostOrder::PHONE_NUMBER_LENGTH_INCORRECT)
+            ->willReturn($violationBuilder);
+        $violationBuilder->atPath('shippingAddress.phoneNumber')
             ->willReturn($violationBuilder);
-        $violationBuilder->setParameter('{{ limit }}', (string) HasValidPhoneNumberInPostOrder::POLISH_PHONE_NUMBER_DEFAULT_LENGTH)
+
+        $violationBuilder->addViolation()->shouldBeCalled();
+
+        $this->initialize($context);
+        $this->validate($value, $constraint);
+    }
+
+    public function it_should_add_violation_if_phone_number_is_too_long_with_polish_long_prefix(
+        ExecutionContextInterface $context,
+        Constraint $constraint,
+        OrderInterface $value,
+        ConstraintViolationBuilderInterface $violationBuilder,
+        ShippingMethodCheckerInterface $shippingMethodChecker,
+    ): void {
+        $addressBuilder = AddressBuilder::create()->withPhoneNumber('+0048 1234567890')->build();
+        $value->implement(InPostPointsAwareInterface::class);
+        $value->getShippingAddress()->willReturn($addressBuilder);
+        $shippingMethodChecker->isInPost(Argument::type(OrderInterface::class))->willReturn(true);
+
+        $context->buildViolation(HasValidPhoneNumberInPostOrder::PHONE_NUMBER_LENGTH_INCORRECT)
             ->willReturn($violationBuilder);
         $violationBuilder->atPath('shippingAddress.phoneNumber')
             ->willReturn($violationBuilder);
@@ -139,13 +227,13 @@ public function it_should_add_violation_if_phone_number_is_too_long(
         $this->validate($value, $constraint);
     }
 
-    public function it_should_do_nothing_if_phone_number_is_valid(
+    public function it_should_do_nothing_if_phone_number_is_valid_without_polish_prefix(
         ExecutionContextInterface $context,
         Constraint $constraint,
         OrderInterface $value,
         ConstraintViolationBuilderInterface $violationBuilder,
         ShippingMethodCheckerInterface $shippingMethodChecker,
-        ): void {
+    ): void {
         $addressBuilder = AddressBuilder::create()->withPhoneNumber('123456789')->build();
         $value->implement(InPostPointsAwareInterface::class);
         $value->getShippingAddress()->willReturn($addressBuilder);
@@ -156,4 +244,94 @@ public function it_should_do_nothing_if_phone_number_is_valid(
         $this->initialize($context);
         $this->validate($value, $constraint);
     }
+
+    public function it_should_do_nothing_if_phone_number_is_valid_with_polish_long_prefix(
+        ExecutionContextInterface $context,
+        Constraint $constraint,
+        OrderInterface $value,
+        ConstraintViolationBuilderInterface $violationBuilder,
+        ShippingMethodCheckerInterface $shippingMethodChecker,
+    ): void {
+        $addressBuilder = AddressBuilder::create()->withPhoneNumber('+0048 123456789')->build();
+        $value->implement(InPostPointsAwareInterface::class);
+        $value->getShippingAddress()->willReturn($addressBuilder);
+        $shippingMethodChecker->isInPost(Argument::type(OrderInterface::class))->willReturn(true);
+
+        $context->buildViolation(Argument::type('string'))->shouldNotBeCalled();
+
+        $this->initialize($context);
+        $this->validate($value, $constraint);
+    }
+
+    public function it_should_do_nothing_if_phone_number_is_valid_with_polish_long_prefix_without_plus(
+        ExecutionContextInterface $context,
+        Constraint $constraint,
+        OrderInterface $value,
+        ConstraintViolationBuilderInterface $violationBuilder,
+        ShippingMethodCheckerInterface $shippingMethodChecker,
+    ): void {
+        $addressBuilder = AddressBuilder::create()->withPhoneNumber('0048 123456789')->build();
+        $value->implement(InPostPointsAwareInterface::class);
+        $value->getShippingAddress()->willReturn($addressBuilder);
+        $shippingMethodChecker->isInPost(Argument::type(OrderInterface::class))->willReturn(true);
+
+        $context->buildViolation(Argument::type('string'))->shouldNotBeCalled();
+
+        $this->initialize($context);
+        $this->validate($value, $constraint);
+    }
+
+    public function it_should_do_nothing_if_phone_number_is_valid_with_polish_prefix_without_plus(
+        ExecutionContextInterface $context,
+        Constraint $constraint,
+        OrderInterface $value,
+        ConstraintViolationBuilderInterface $violationBuilder,
+        ShippingMethodCheckerInterface $shippingMethodChecker,
+    ): void {
+        $addressBuilder = AddressBuilder::create()->withPhoneNumber('48 123456789')->build();
+        $value->implement(InPostPointsAwareInterface::class);
+        $value->getShippingAddress()->willReturn($addressBuilder);
+        $shippingMethodChecker->isInPost(Argument::type(OrderInterface::class))->willReturn(true);
+
+        $context->buildViolation(Argument::type('string'))->shouldNotBeCalled();
+
+        $this->initialize($context);
+        $this->validate($value, $constraint);
+    }
+
+    public function it_should_do_nothing_if_phone_number_is_valid_with_polish_prefix_without_plus_with_dot(
+        ExecutionContextInterface $context,
+        Constraint $constraint,
+        OrderInterface $value,
+        ConstraintViolationBuilderInterface $violationBuilder,
+        ShippingMethodCheckerInterface $shippingMethodChecker,
+    ): void {
+        $addressBuilder = AddressBuilder::create()->withPhoneNumber('48.123456789')->build();
+        $value->implement(InPostPointsAwareInterface::class);
+        $value->getShippingAddress()->willReturn($addressBuilder);
+        $shippingMethodChecker->isInPost(Argument::type(OrderInterface::class))->willReturn(true);
+
+        $context->buildViolation(Argument::type('string'))->shouldNotBeCalled();
+
+        $this->initialize($context);
+        $this->validate($value, $constraint);
+    }
+
+    public function it_should_do_nothing_if_phone_number_is_valid_with_polish_prefix_with_plus_and_dot(
+        ExecutionContextInterface $context,
+        Constraint $constraint,
+        OrderInterface $value,
+        ConstraintViolationBuilderInterface $violationBuilder,
+        ShippingMethodCheckerInterface $shippingMethodChecker,
+    ): void {
+        $addressBuilder = AddressBuilder::create()->withPhoneNumber('+48.123456789')->build();
+        $value->implement(InPostPointsAwareInterface::class);
+        $value->getShippingAddress()->willReturn($addressBuilder);
+        $shippingMethodChecker->isInPost(Argument::type(OrderInterface::class))->willReturn(true);
+
+        $context->buildViolation(Argument::type('string'))->shouldNotBeCalled();
+
+        $this->initialize($context);
+        $this->validate($value, $constraint);
+    }
 }
diff --git a/src/Resources/translations/validators.en.yml b/src/Resources/translations/validators.en.yml
@@ -3,5 +3,4 @@ bitbag_sylius_inpost_plugin:
         phone_number_is_required: 'Phone number is required in order to use InPost shipping method.'
         not_allowed_payment_method: 'Cash on delivery is not allowed to use with InPost shipping method.'
         phone_number:
-            min: 'Phone number must be at least {{ limit }} characters long'
-            max: 'Phone  number cannot be longer than {{ limit }} characters long'
+            incorrect_length: "Incorrect phone number for Polish shipping methods"
diff --git a/src/Validator/Constraint/HasValidPhoneNumberInPostOrder.php b/src/Validator/Constraint/HasValidPhoneNumberInPostOrder.php
@@ -14,12 +14,19 @@
 
 final class HasValidPhoneNumberInPostOrder extends Constraint
 {
-    public const PHONE_NUMBER_IS_TOO_SHORT_MESSAGE = 'bitbag_sylius_inpost_plugin.order.phone_number.min';
-
-    public const PHONE_NUMBER_IS_TOO_LONG_MESSAGE = 'bitbag_sylius_inpost_plugin.order.phone_number.max';
+    public const PHONE_NUMBER_LENGTH_INCORRECT = 'bitbag_sylius_inpost_plugin.order.phone_number.incorrect_length';
 
     public const POLISH_PHONE_NUMBER_DEFAULT_LENGTH = 9;
 
+    public const POSSIBLE_POLISH_PHONE_PREFIXES = [
+        '+0048',
+        '0048',
+        '+48.',
+        '+48',
+        '48.',
+        '48',
+    ];
+
     public function validatedBy(): string
     {
         return 'bitbag_sylius_inpost_plugin_validator_has_valid_phone_number_inpost_order';

diff --git a/src/Validator/HasValidPhoneNumberInPostOrderValidator.php b/src/Validator/HasValidPhoneNumberInPostOrderValidator.php
@@ -44,21 +44,32 @@ public function validate($value, Constraint $constraint): void
             return;
         }
 
-        $length = strlen($phone);
-        if (HasValidPhoneNumberInPostOrder::POLISH_PHONE_NUMBER_DEFAULT_LENGTH > $length) {
-            $this->addPhoneNumberViolation(HasValidPhoneNumberInPostOrder::PHONE_NUMBER_IS_TOO_SHORT_MESSAGE);
-        }
+        $formatedPhone = $this->formatPhoneNumber($phone);
+
+        $length = strlen($formatedPhone);
 
-        if (HasValidPhoneNumberInPostOrder::POLISH_PHONE_NUMBER_DEFAULT_LENGTH < $length) {
-            $this->addPhoneNumberViolation(HasValidPhoneNumberInPostOrder::PHONE_NUMBER_IS_TOO_LONG_MESSAGE);
+        if (HasValidPhoneNumberInPostOrder::POLISH_PHONE_NUMBER_DEFAULT_LENGTH !== $length) {
+            $this->addPhoneNumberViolation(HasValidPhoneNumberInPostOrder::PHONE_NUMBER_LENGTH_INCORRECT);
         }
     }
 
     private function addPhoneNumberViolation(string $message): void
     {
         $violationBuilder = $this->context->buildViolation($message);
-        $violationBuilder->setParameter('{{ limit }}', (string) HasValidPhoneNumberInPostOrder::POLISH_PHONE_NUMBER_DEFAULT_LENGTH);
         $violationBuilder->atPath('shippingAddress.phoneNumber')
             ->addViolation();
     }
+
+    private function formatPhoneNumber($phone): string
+    {
+        $phone = preg_replace('/\s+/', '', $phone);
+
+        if (9 < strlen($phone)) {
+            $escaped_prefixes = array_map('preg_quote', HasValidPhoneNumberInPostOrder::POSSIBLE_POLISH_PHONE_PREFIXES);
+            $pattern = '/^(' . implode('|', $escaped_prefixes) . ')/';
+            $phone = preg_replace($pattern, '', $phone);
+        }
+
+        return (string) $phone;
+    }
 }