From 75fd0ae9544cc36791584ce48fe09a9fa4eafd82 Mon Sep 17 00:00:00 2001 From: Szymon Kostrubiec Date: Wed, 8 May 2024 11:53:16 +0200 Subject: [PATCH 01/13] working flashbag --- src/EventListener/ShippingExportEventListener.php | 14 +++++++------- .../InPostShippingExportConfirmedAction.php | 10 +++++----- .../InPostShippingExportDefaultAction.php | 10 +++++----- src/Resources/config/services.xml | 2 +- .../services/inpost_shipping_export_action.xml | 4 ++-- tests/Application/config/services.yaml | 4 ---- 6 files changed, 20 insertions(+), 24 deletions(-) diff --git a/src/EventListener/ShippingExportEventListener.php b/src/EventListener/ShippingExportEventListener.php index 2b5e6e9..090fef8 100644 --- a/src/EventListener/ShippingExportEventListener.php +++ b/src/EventListener/ShippingExportEventListener.php @@ -18,7 +18,7 @@ use Psr\Log\LoggerInterface; use Sylius\Bundle\ResourceBundle\Event\ResourceControllerEvent; use Sylius\Component\Core\Model\ShipmentInterface; -use Symfony\Component\HttpFoundation\Session\Flash\FlashBagInterface; +use Symfony\Component\HttpFoundation\RequestStack; use Webmozart\Assert\Assert; final class ShippingExportEventListener @@ -35,19 +35,19 @@ final class ShippingExportEventListener private InPostShippingExportActionProviderInterface $shippingExportActionProvider; - private FlashBagInterface $flashBag; + private RequestStack $requestStack; private LoggerInterface $logger; public function __construct( WebClientInterface $webClient, InPostShippingExportActionProviderInterface $shippingExportActionProvider, - FlashBagInterface $flashBag, + RequestStack $requestStack, LoggerInterface $logger ) { $this->webClient = $webClient; $this->shippingExportActionProvider = $shippingExportActionProvider; - $this->flashBag = $flashBag; + $this->requestStack = $requestStack; $this->logger = $logger; } @@ -74,7 +74,7 @@ public function exportShipment(ResourceControllerEvent $exportShipmentEvent): vo try { $createShipmentResponse = $this->webClient->createShipment($shipment); } catch (ClientException $exception) { - $this->flashBag->add('error', 'bitbag.ui.shipping_export_error'); + $this->requestStack->getSession()->getFlashBag()->add('error', 'bitbag.ui.shipping_export_error'); $this->logError($exception, $shipment); return; @@ -86,7 +86,7 @@ public function exportShipment(ResourceControllerEvent $exportShipmentEvent): vo try { $shipmentData = $this->webClient->getShipmentById((int) ($shippingExport->getExternalId())); } catch (ClientException $exception) { - $this->flashBag->add('error', 'bitbag.ui.shipping_export_error'); + $this->requestStack->getSession()->getFlashBag()->add('error', 'bitbag.ui.shipping_export_error'); $this->logError($exception, $shipment); return; @@ -100,7 +100,7 @@ public function exportShipment(ResourceControllerEvent $exportShipmentEvent): vo try { $action = $this->shippingExportActionProvider->provide($status); } catch (\Exception $exception) { - $this->flashBag->add('error', 'bitbag.ui.shipping_export_error'); + $this->requestStack->getSession()->getFlashBag()->add('error', 'bitbag.ui.shipping_export_error'); $this->logError($exception, $shipment); return; diff --git a/src/EventListener/ShippingExportEventListener/InPostShippingExportConfirmedAction.php b/src/EventListener/ShippingExportEventListener/InPostShippingExportConfirmedAction.php index 3ba9174..c9888a3 100644 --- a/src/EventListener/ShippingExportEventListener/InPostShippingExportConfirmedAction.php +++ b/src/EventListener/ShippingExportEventListener/InPostShippingExportConfirmedAction.php @@ -17,7 +17,7 @@ use Sylius\Component\Core\Model\OrderInterface; use Sylius\Component\Core\Model\ShipmentInterface; use Symfony\Component\Filesystem\Filesystem; -use Symfony\Component\HttpFoundation\Session\Flash\FlashBagInterface; +use Symfony\Component\HttpFoundation\RequestStack; use Webmozart\Assert\Assert; final class InPostShippingExportConfirmedAction implements InPostShippingExportActionInterface @@ -36,7 +36,7 @@ final class InPostShippingExportConfirmedAction implements InPostShippingExportA private Filesystem $filesystem; - private FlashBagInterface $flashBag; + private RequestStack $requestStack; private string $shippingLabelsPath; @@ -44,13 +44,13 @@ public function __construct( ShippingExportRepositoryInterface $shippingExportRepository, WebClientInterface $webClient, Filesystem $filesystem, - FlashBagInterface $flashBag, + RequestStack $requestStack, string $shippingLabelsPath ) { $this->shippingExportRepository = $shippingExportRepository; $this->webClient = $webClient; $this->filesystem = $filesystem; - $this->flashBag = $flashBag; + $this->requestStack = $requestStack; $this->shippingLabelsPath = $shippingLabelsPath; } @@ -72,7 +72,7 @@ public function execute(ShippingExportInterface $shippingExport): void $shippingExport->setExportedAt(new \DateTime()); $this->shippingExportRepository->add($shippingExport); - $this->flashBag->add(self::SUCCESS, self::TRANSLATION_KEY); + $this->requestStack->getSession()->getFlashBag()->add(self::SUCCESS, self::TRANSLATION_KEY); } private function validateShippingExport(ShippingExportInterface $shippingExport): void diff --git a/src/EventListener/ShippingExportEventListener/InPostShippingExportDefaultAction.php b/src/EventListener/ShippingExportEventListener/InPostShippingExportDefaultAction.php index 3c0c091..bd51fc1 100644 --- a/src/EventListener/ShippingExportEventListener/InPostShippingExportDefaultAction.php +++ b/src/EventListener/ShippingExportEventListener/InPostShippingExportDefaultAction.php @@ -12,7 +12,7 @@ use BitBag\SyliusShippingExportPlugin\Entity\ShippingExportInterface; use BitBag\SyliusShippingExportPlugin\Repository\ShippingExportRepositoryInterface; -use Symfony\Component\HttpFoundation\Session\Flash\FlashBagInterface; +use Symfony\Component\HttpFoundation\RequestStack; final class InPostShippingExportDefaultAction implements InPostShippingExportActionInterface { @@ -22,12 +22,12 @@ final class InPostShippingExportDefaultAction implements InPostShippingExportAct private ShippingExportRepositoryInterface $shippingExportRepository; - private FlashBagInterface $flashBag; + private RequestStack $requestStack; - public function __construct(ShippingExportRepositoryInterface $shippingExportRepository, FlashBagInterface $flashBag) + public function __construct(ShippingExportRepositoryInterface $shippingExportRepository, RequestStack $requestStack) { $this->shippingExportRepository = $shippingExportRepository; - $this->flashBag = $flashBag; + $this->requestStack = $requestStack; } public function execute(ShippingExportInterface $shippingExport): void @@ -37,7 +37,7 @@ public function execute(ShippingExportInterface $shippingExport): void $this->shippingExportRepository->add($shippingExport); - $this->flashBag->add(self::INFO, self::TRANSLATION_KEY); + $this->requestStack->getSession()->getFlashBag()->add(self::INFO, self::TRANSLATION_KEY); } public function supports(string $statusCode): bool diff --git a/src/Resources/config/services.xml b/src/Resources/config/services.xml index 6257360..b284702 100644 --- a/src/Resources/config/services.xml +++ b/src/Resources/config/services.xml @@ -19,7 +19,7 @@ > - + diff --git a/src/Resources/config/services/inpost_shipping_export_action.xml b/src/Resources/config/services/inpost_shipping_export_action.xml index 2521c19..25209d0 100644 --- a/src/Resources/config/services/inpost_shipping_export_action.xml +++ b/src/Resources/config/services/inpost_shipping_export_action.xml @@ -16,7 +16,7 @@ - + %bitbag.shipping_labels_path% @@ -27,7 +27,7 @@ class="BitBag\SyliusInPostPlugin\EventListener\ShippingExportEventListener\InPostShippingExportDefaultAction" > - + diff --git a/tests/Application/config/services.yaml b/tests/Application/config/services.yaml index 0a54dee..615506e 100644 --- a/tests/Application/config/services.yaml +++ b/tests/Application/config/services.yaml @@ -2,7 +2,3 @@ # https://symfony.com/doc/current/best_practices/configuration.html#application-related-configuration parameters: locale: en_US - -services: - session.flash_bag: - class: Symfony\Component\HttpFoundation\Session\Flash\FlashBagInterface From 2bc6378422d000ac294a10251123550225da4bcf Mon Sep 17 00:00:00 2001 From: Szymon Kostrubiec Date: Wed, 8 May 2024 15:22:58 +0200 Subject: [PATCH 02/13] op-264 Fix flashbag, ECS --- spec/Api/WebClientSpec.php | 10 ++-- spec/Checker/ShippingMethodCheckerSpec.php | 8 +-- ...InPostShippingExportActionProviderSpec.php | 8 +-- ...nPostShippingExportConfirmedActionSpec.php | 44 +++++++++------ .../InPostShippingExportDefaultActionSpec.php | 33 ++++++++---- .../ShippingExportEventListenerSpec.php | 54 ++++++++++++------- ...dPaymentMethodInPostOrderValidatorSpec.php | 17 +++--- ...HasPhoneNumberInPostOrderValidatorSpec.php | 17 +++--- .../ShippingExportEventListener.php | 6 +-- .../InPostShippingExportConfirmedAction.php | 2 +- .../InPostShippingExportDefaultAction.php | 2 +- 11 files changed, 118 insertions(+), 83 deletions(-) diff --git a/spec/Api/WebClientSpec.php b/spec/Api/WebClientSpec.php index 8f34c8c..7ec3831 100644 --- a/spec/Api/WebClientSpec.php +++ b/spec/Api/WebClientSpec.php @@ -18,18 +18,18 @@ final class WebClientSpec extends ObjectBehavior { - const ORGANIZATION_ID = '123456'; + public const ORGANIZATION_ID = '123456'; - const API_ENDPOINT = 'https://api-shipx-pl.easypack24.net/v1'; + public const API_ENDPOINT = 'https://api-shipx-pl.easypack24.net/v1'; - const POINT_NAME = 'AAA666'; + public const POINT_NAME = 'AAA666'; - function let(Client $client): void + public function let(Client $client): void { $this->beConstructedWith($client); } - function it_is_initializable(): void + public function it_is_initializable(): void { $this->shouldHaveType(WebClient::class); $this->shouldImplement(WebClientInterface::class); diff --git a/spec/Checker/ShippingMethodCheckerSpec.php b/spec/Checker/ShippingMethodCheckerSpec.php index 9122b5f..abe8aaf 100644 --- a/spec/Checker/ShippingMethodCheckerSpec.php +++ b/spec/Checker/ShippingMethodCheckerSpec.php @@ -22,13 +22,13 @@ class ShippingMethodCheckerSpec extends ObjectBehavior { public const NOT_INPOST = 'not_inpost'; - function it_is_initializable(): void + public function it_is_initializable(): void { $this->shouldHaveType(ShippingMethodChecker::class); $this->shouldBeAnInstanceOf(ShippingMethodCheckerInterface::class); } - function it_should_return_true_if_shipping_method_is_inpost(): void + public function it_should_return_true_if_shipping_method_is_inpost(): void { $shippingMethod = ShippingMethodBuilder::create() ->withCode(ShippingExportEventListener::INPOST_SHIPPING_GATEWAY_CODE) @@ -39,7 +39,7 @@ function it_should_return_true_if_shipping_method_is_inpost(): void $this->isInPost($order)->shouldReturn(true); } - function it_should_return_true_if_shipping_method_is_inpost_point(): void + public function it_should_return_true_if_shipping_method_is_inpost_point(): void { $shippingMethod = ShippingMethodBuilder::create() ->withCode(ShippingExportEventListener::INPOST_POINT_SHIPPING_GATEWAY_CODE) @@ -50,7 +50,7 @@ function it_should_return_true_if_shipping_method_is_inpost_point(): void $this->isInPost($order)->shouldReturn(true); } - function it_should_return_false_if_shipping_method_is_not_inpost(): void + public function it_should_return_false_if_shipping_method_is_not_inpost(): void { $shippingMethod = ShippingMethodBuilder::create() ->withCode(self::NOT_INPOST) diff --git a/spec/EventListener/ShippingExportEventListener/InPostShippingExportActionProviderSpec.php b/spec/EventListener/ShippingExportEventListener/InPostShippingExportActionProviderSpec.php index c9cae05..0a24522 100644 --- a/spec/EventListener/ShippingExportEventListener/InPostShippingExportActionProviderSpec.php +++ b/spec/EventListener/ShippingExportEventListener/InPostShippingExportActionProviderSpec.php @@ -20,20 +20,20 @@ final class InPostShippingExportActionProviderSpec extends ObjectBehavior { private const FOO = 'foo'; - function let( + public function let( InPostShippingExportActionInterface $action1, InPostShippingExportActionInterface $action2 ): void { $this->beConstructedWith([$action1, $action2]); } - function it_is_initializable(): void + public function it_is_initializable(): void { $this->shouldHaveType(InPostShippingExportActionProvider::class); $this->shouldBeAnInstanceOf(InPostShippingExportActionProviderInterface::class); } - function it_should_throw_exception_if_no_action_supports_passed_status_code( + public function it_should_throw_exception_if_no_action_supports_passed_status_code( InPostShippingExportActionInterface $action1, InPostShippingExportActionInterface $action2 ): void { @@ -43,7 +43,7 @@ function it_should_throw_exception_if_no_action_supports_passed_status_code( $this->shouldThrow(\InvalidArgumentException::class)->during('provide', [self::FOO]); } - function it_should_return_first_supported_action( + public function it_should_return_first_supported_action( InPostShippingExportActionInterface $action1, InPostShippingExportActionInterface $action2 ): void { diff --git a/spec/EventListener/ShippingExportEventListener/InPostShippingExportConfirmedActionSpec.php b/spec/EventListener/ShippingExportEventListener/InPostShippingExportConfirmedActionSpec.php index 1427382..16121f1 100644 --- a/spec/EventListener/ShippingExportEventListener/InPostShippingExportConfirmedActionSpec.php +++ b/spec/EventListener/ShippingExportEventListener/InPostShippingExportConfirmedActionSpec.php @@ -19,7 +19,9 @@ use Prophecy\Argument; use Sylius\Component\Core\Model\ShipmentInterface; use Symfony\Component\Filesystem\Filesystem; +use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\HttpFoundation\Session\Flash\FlashBagInterface; +use Symfony\Component\HttpFoundation\Session\SessionInterface; use Tests\BitBag\SyliusInPostPlugin\Spec\Builder\OrderBuilder; use Tests\BitBag\SyliusInPostPlugin\Spec\Builder\ShipmentBuilder; use Tests\BitBag\SyliusInPostPlugin\Spec\Builder\ShippingExportBuilder; @@ -39,32 +41,32 @@ class InPostShippingExportConfirmedActionSpec extends ObjectBehavior private const ORDER_NUMBER = '0000000001'; - function let( + public function let( ShippingExportRepositoryInterface $shippingExportRepository, WebClientInterface $webClient, Filesystem $filesystem, - FlashBagInterface $flashBag + RequestStack $requestStack ): void { - $this->beConstructedWith($shippingExportRepository, $webClient, $filesystem, $flashBag, self::SHIPPING_LABELS_PATH); + $this->beConstructedWith($shippingExportRepository, $webClient, $filesystem, $requestStack, self::SHIPPING_LABELS_PATH); } - function it_is_initializable(): void + public function it_is_initializable(): void { $this->shouldHaveType(InPostShippingExportConfirmedAction::class); $this->shouldBeAnInstanceOf(InPostShippingExportActionInterface::class); } - function it_should_support_the_correct_status_code(): void + public function it_should_support_the_correct_status_code(): void { $this->supports(self::CONFIRMED)->shouldReturn(true); } - function it_should_not_support_the_incorrect_status_code(): void + public function it_should_not_support_the_incorrect_status_code(): void { $this->supports(self::FOO)->shouldReturn(false); } - function it_should_throw_exception_if_shipment_external_id_is_null(): void + public function it_should_throw_exception_if_shipment_external_id_is_null(): void { $shippingExport = ShippingExportBuilder::create()->build(); $this @@ -72,7 +74,7 @@ function it_should_throw_exception_if_shipment_external_id_is_null(): void ->during('execute', [$shippingExport]); } - function it_should_throw_exception_if_shipment_is_null(): void + public function it_should_throw_exception_if_shipment_is_null(): void { $shippingExport = ShippingExportBuilder::create()->withExternalId(self::FOO)->build(); @@ -81,7 +83,7 @@ function it_should_throw_exception_if_shipment_is_null(): void ->during('execute', [$shippingExport]); } - function it_should_throw_exception_if_order_is_null(): void + public function it_should_throw_exception_if_order_is_null(): void { $shipment = ShipmentBuilder::create()->build(); $shippingExport = ShippingExportBuilder::create() @@ -94,7 +96,7 @@ function it_should_throw_exception_if_order_is_null(): void ->during('execute', [$shippingExport]); } - function it_should_throw_exception_if_order_number_is_null(): void + public function it_should_throw_exception_if_order_number_is_null(): void { $order = OrderBuilder::create()->build(); $shipment = ShipmentBuilder::create()->withOrder($order)->build(); @@ -108,11 +110,14 @@ function it_should_throw_exception_if_order_number_is_null(): void ->during('execute', [$shippingExport]); } - function it_should_save_label_under_the_correct_path( + public function it_should_save_label_under_the_correct_path( WebClientInterface $webClient, ShipmentInterface $shipment, - Filesystem $filesystem - ): void { + Filesystem $filesystem, + RequestStack $requestStack, + SessionInterface $session, + FlashBagInterface $flashBag + ): void { $order = OrderBuilder::create()->withNumber(self::ORDER_NUMBER)->build(); $shipment->getOrder()->willReturn($order); $shipment->getId()->willReturn(self::SHIPMENT_ID); @@ -125,16 +130,21 @@ function it_should_save_label_under_the_correct_path( $expectedPath = sprintf('%s/%s_%s.pdf', self::SHIPPING_LABELS_PATH, self::SHIPMENT_ID, self::ORDER_NUMBER); $filesystem->dumpFile($expectedPath, self::SHIPPING_LABEL_CONTENT)->shouldBeCalled(); + $requestStack->getSession()->willReturn($session); + $session->getBag('flashes')->willReturn($flashBag); $this->execute($shippingExport); } - function it_should_update_shipping_export_entity( + public function it_should_update_shipping_export_entity( WebClientInterface $webClient, ShippingExportRepositoryInterface $shippingExportRepository, ShipmentInterface $shipment, - ShippingExportInterface $shippingExport - ): void { + ShippingExportInterface $shippingExport, + RequestStack $requestStack, + SessionInterface $session, + FlashBagInterface $flashBag + ): void { $webClient->getLabels([self::FOO])->willReturn(self::SHIPPING_LABEL_CONTENT); $order = OrderBuilder::create()->withNumber(self::ORDER_NUMBER)->build(); $shipment->getOrder()->willReturn($order); @@ -146,6 +156,8 @@ function it_should_update_shipping_export_entity( $shippingExport->setState(ShippingExportInterface::STATE_EXPORTED)->shouldBeCalled(); $shippingExport->setExportedAt(Argument::type(\DateTime::class))->shouldBeCalled(); $shippingExportRepository->add($shippingExport)->shouldBeCalled(); + $requestStack->getSession()->willReturn($session); + $session->getBag('flashes')->willReturn($flashBag); $this->execute($shippingExport); } diff --git a/spec/EventListener/ShippingExportEventListener/InPostShippingExportDefaultActionSpec.php b/spec/EventListener/ShippingExportEventListener/InPostShippingExportDefaultActionSpec.php index b0d3d38..462046f 100644 --- a/spec/EventListener/ShippingExportEventListener/InPostShippingExportDefaultActionSpec.php +++ b/spec/EventListener/ShippingExportEventListener/InPostShippingExportDefaultActionSpec.php @@ -16,41 +16,56 @@ use BitBag\SyliusShippingExportPlugin\Repository\ShippingExportRepositoryInterface; use PhpSpec\ObjectBehavior; use Prophecy\Argument; +use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\HttpFoundation\Session\Flash\FlashBagInterface; +use Symfony\Component\HttpFoundation\Session\SessionInterface; class InPostShippingExportDefaultActionSpec extends ObjectBehavior { private const FOO = 'foo'; - function let(ShippingExportRepositoryInterface $shippingExportRepository, FlashBagInterface $flashBag): void + public function let(ShippingExportRepositoryInterface $shippingExportRepository, RequestStack $requestStack): void { - $this->beConstructedWith($shippingExportRepository, $flashBag); + $this->beConstructedWith($shippingExportRepository, $requestStack); } - function it_is_initializable() + public function it_is_initializable() { $this->shouldHaveType(InPostShippingExportDefaultAction::class); $this->shouldBeAnInstanceOf(InPostShippingExportActionInterface::class); } - function it_should_set_state_and_exported_at_properties(ShippingExportInterface $shippingExport): void - { + public function it_should_set_state_and_exported_at_properties( + ShippingExportInterface $shippingExport, + RequestStack $requestStack, + SessionInterface $session, + FlashBagInterface $flashBag + ): void { $shippingExport->setState(ShippingExportInterface::STATE_PENDING)->shouldBeCalled(); $shippingExport->setExportedAt(Argument::type(\DateTime::class))->shouldBeCalled(); + $requestStack->getSession()->willReturn($session); + $session->getBag('flashes')->willReturn($flashBag); + $this->execute($shippingExport); } - function it_should_save_shipping_export_changes( + public function it_should_save_shipping_export_changes( ShippingExportInterface $shippingExport, - ShippingExportRepositoryInterface $shippingExportRepository - ): void { + ShippingExportRepositoryInterface $shippingExportRepository, + RequestStack $requestStack, + SessionInterface $session, + FlashBagInterface $flashBag + ): void { $shippingExportRepository->add($shippingExport)->shouldBeCalled(); + $requestStack->getSession()->willReturn($session); + $session->getBag('flashes')->willReturn($flashBag); + $this->execute($shippingExport); } - function it_should_always_support(): void + public function it_should_always_support(): void { $this->supports(self::FOO)->shouldReturn(true); } diff --git a/spec/EventListener/ShippingExportEventListenerSpec.php b/spec/EventListener/ShippingExportEventListenerSpec.php index 6a1ad21..c5c8494 100644 --- a/spec/EventListener/ShippingExportEventListenerSpec.php +++ b/spec/EventListener/ShippingExportEventListenerSpec.php @@ -19,7 +19,9 @@ use Prophecy\Argument; use Psr\Log\LoggerInterface; use Sylius\Bundle\ResourceBundle\Event\ResourceControllerEvent; +use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\HttpFoundation\Session\Flash\FlashBagInterface; +use Symfony\Component\HttpFoundation\Session\SessionInterface; use Tests\BitBag\SyliusInPostPlugin\Spec\Builder\OrderBuilder; use Tests\BitBag\SyliusInPostPlugin\Spec\Builder\ShipmentBuilder; use Tests\BitBag\SyliusInPostPlugin\Spec\Builder\ShippingExportBuilder; @@ -46,21 +48,21 @@ final class ShippingExportEventListenerSpec extends ObjectBehavior 'status' => 'created', ]; - function let( + public function let( WebClientInterface $webClient, InPostShippingExportActionProviderInterface $shippingExportActionProvider, - FlashBagInterface $flashBag, + RequestStack $requestStack, LoggerInterface $logger ): void { - $this->beConstructedWith($webClient, $shippingExportActionProvider, $flashBag, $logger); + $this->beConstructedWith($webClient, $shippingExportActionProvider, $requestStack, $logger); } - function it_is_initializable(): void + public function it_is_initializable(): void { $this->shouldHaveType(ShippingExportEventListener::class); } - function it_should_throw_exception_if_subject_is_not_shipping_export_instance(ResourceControllerEvent $event): void + public function it_should_throw_exception_if_subject_is_not_shipping_export_instance(ResourceControllerEvent $event): void { $event->getSubject()->willReturn(new \stdClass()); @@ -70,7 +72,7 @@ function it_should_throw_exception_if_subject_is_not_shipping_export_instance(Re ; } - function it_should_throw_exception_if_shipping_gateway_is_null(ResourceControllerEvent $event): void + public function it_should_throw_exception_if_shipping_gateway_is_null(ResourceControllerEvent $event): void { $shippingExport = ShippingExportBuilder::create()->build(); $event->getSubject()->willReturn($shippingExport); @@ -80,7 +82,7 @@ function it_should_throw_exception_if_shipping_gateway_is_null(ResourceControlle ->during('exportShipment', [$event]); } - function it_should_throw_exception_if_shipment_is_null(ResourceControllerEvent $event): void + public function it_should_throw_exception_if_shipment_is_null(ResourceControllerEvent $event): void { $shippingGateway = ShippingGatewayBuilder::create()->build(); $shippingExport = ShippingExportBuilder::create()->withShippingGateway($shippingGateway)->build(); @@ -91,7 +93,7 @@ function it_should_throw_exception_if_shipment_is_null(ResourceControllerEvent $ ->during('exportShipment', [$event]); } - function it_should_do_nothing_if_shipping_gateway_code_is_not_inpost( + public function it_should_do_nothing_if_shipping_gateway_code_is_not_inpost( ResourceControllerEvent $event, WebClientInterface $webClient ): void { @@ -108,12 +110,14 @@ function it_should_do_nothing_if_shipping_gateway_code_is_not_inpost( $this->exportShipment($event); } - function it_should_handle_when_create_shipment_throws_exception( + public function it_should_handle_when_create_shipment_throws_exception( ResourceControllerEvent $event, WebClientInterface $webClient, FlashBagInterface $flashBag, - LoggerInterface $logger - ): void { + LoggerInterface $logger, + RequestStack $requestStack, + SessionInterface $session, + ): void { $order = OrderBuilder::create()->withNumber(self::ORDER_NUMBER)->build(); $shippingGateway = ShippingGatewayBuilder::create()->withCode(self::INPOST)->build(); $shipment = ShipmentBuilder::create()->withOrder($order)->build(); @@ -125,18 +129,22 @@ function it_should_handle_when_create_shipment_throws_exception( $webClient->setShippingGateway($shippingGateway); $webClient->createShipment($shipment)->willThrow(ClientException::class); + $requestStack->getSession()->willReturn($session); + $session->getBag('flashes')->willReturn($flashBag); $flashBag->add(self::ERROR, self::SHIPPING_EXPORT_ERROR_MESSAGE)->shouldBeCalled(); $logger->error(Argument::type('string'))->shouldBeCalled(); $this->exportShipment($event); } - function it_should_handle_when_get_shipment_by_id_throws_exception( + public function it_should_handle_when_get_shipment_by_id_throws_exception( ResourceControllerEvent $event, WebClientInterface $webClient, FlashBagInterface $flashBag, - LoggerInterface $logger - ): void { + LoggerInterface $logger, + RequestStack $requestStack, + SessionInterface $session, + ): void { $order = OrderBuilder::create()->withNumber(self::ORDER_NUMBER)->build(); $shippingGateway = ShippingGatewayBuilder::create()->withCode(self::INPOST)->build(); $shipment = ShipmentBuilder::create()->withOrder($order)->build(); @@ -149,19 +157,23 @@ function it_should_handle_when_get_shipment_by_id_throws_exception( $webClient->setShippingGateway($shippingGateway); $webClient->createShipment($shipment)->willReturn(self::EXPECTED_CREATE_SHIPMENT_RESPONSE); $webClient->getShipmentById(self::SHIPMENT_ID)->willThrow(ClientException::class); + $requestStack->getSession()->willReturn($session); + $session->getBag('flashes')->willReturn($flashBag); $flashBag->add(self::ERROR, self::SHIPPING_EXPORT_ERROR_MESSAGE)->shouldBeCalled(); $logger->error(Argument::type('string'))->shouldBeCalled(); $this->exportShipment($event); } - function it_should_handle_when_provide_shipment_action_throws_exception( + public function it_should_handle_when_provide_shipment_action_throws_exception( ResourceControllerEvent $event, WebClientInterface $webClient, FlashBagInterface $flashBag, LoggerInterface $logger, - InPostShippingExportActionProviderInterface $shippingExportActionProvider - ): void { + InPostShippingExportActionProviderInterface $shippingExportActionProvider, + RequestStack $requestStack, + SessionInterface $session, + ): void { $order = OrderBuilder::create()->withNumber(self::ORDER_NUMBER)->build(); $shippingGateway = ShippingGatewayBuilder::create()->withCode(self::INPOST)->build(); $shipment = ShipmentBuilder::create()->withOrder($order)->build(); @@ -175,18 +187,20 @@ function it_should_handle_when_provide_shipment_action_throws_exception( $webClient->createShipment($shipment)->willReturn(self::EXPECTED_CREATE_SHIPMENT_RESPONSE); $webClient->getShipmentById(self::SHIPMENT_ID)->willReturn(self::EXPECTED_GET_SHIPMENT_RESPONSE); $shippingExportActionProvider->provide(Argument::type('string'))->willThrow(\Exception::class); + $requestStack->getSession()->willReturn($session); + $session->getBag('flashes')->willReturn($flashBag); $flashBag->add(self::ERROR, self::SHIPPING_EXPORT_ERROR_MESSAGE)->shouldBeCalled(); $logger->error(Argument::type('string'))->shouldBeCalled(); $this->exportShipment($event); } - function it_should_execute_shipping_export_action( + public function it_should_execute_shipping_export_action( ResourceControllerEvent $event, WebClientInterface $webClient, InPostShippingExportActionProviderInterface $shippingExportActionProvider, - InPostShippingExportActionInterface $shippingExportAction - ): void { + InPostShippingExportActionInterface $shippingExportAction, + ): void { $shippingGateway = ShippingGatewayBuilder::create()->withCode(self::INPOST)->build(); $shipment = ShipmentBuilder::create()->build(); $shippingExport = ShippingExportBuilder::create() diff --git a/spec/Validator/HasAllowedPaymentMethodInPostOrderValidatorSpec.php b/spec/Validator/HasAllowedPaymentMethodInPostOrderValidatorSpec.php index 766abf9..9ef1f4c 100644 --- a/spec/Validator/HasAllowedPaymentMethodInPostOrderValidatorSpec.php +++ b/spec/Validator/HasAllowedPaymentMethodInPostOrderValidatorSpec.php @@ -25,25 +25,23 @@ use Symfony\Component\Validator\Violation\ConstraintViolationBuilderInterface; use Tests\BitBag\SyliusInPostPlugin\Spec\Builder\PaymentBuilder; use Tests\BitBag\SyliusInPostPlugin\Spec\Builder\PaymentMethodBuilder; -use Tests\BitBag\SyliusInPostPlugin\Spec\ObjectMother\PaymentObjectMother; -use Tests\BitBag\SyliusInPostPlugin\Spec\ObjectMother\ShipmentObjectMother; class HasAllowedPaymentMethodInPostOrderValidatorSpec extends ObjectBehavior { public const FOO = 'foo'; - function let(ShippingMethodCheckerInterface $shippingMethodChecker): void + public function let(ShippingMethodCheckerInterface $shippingMethodChecker): void { $this->beConstructedWith($shippingMethodChecker); } - function it_is_initializable(): void + public function it_is_initializable(): void { $this->shouldHaveType(HasAllowedPaymentMethodInPostOrderValidator::class); $this->shouldBeAnInstanceOf(ConstraintValidator::class); } - function it_should_do_nothing_if_value_is_not_instance_of_order_interface( + public function it_should_do_nothing_if_value_is_not_instance_of_order_interface( ExecutionContextInterface $context, Constraint $constraint ): void { @@ -54,7 +52,7 @@ function it_should_do_nothing_if_value_is_not_instance_of_order_interface( $this->validate($value, $constraint); } - function it_should_do_nothing_if_value_is_not_implementing_inpost_points_aware_interface( + public function it_should_do_nothing_if_value_is_not_implementing_inpost_points_aware_interface( ExecutionContextInterface $context, Constraint $constraint, OrderInterface $value @@ -65,7 +63,7 @@ function it_should_do_nothing_if_value_is_not_implementing_inpost_points_aware_i $this->validate($value, $constraint); } - function it_should_do_nothing_if_selected_shipping_method_is_not_inpost( + public function it_should_do_nothing_if_selected_shipping_method_is_not_inpost( ExecutionContextInterface $context, Constraint $constraint, OrderInterface $value, @@ -79,14 +77,13 @@ function it_should_do_nothing_if_selected_shipping_method_is_not_inpost( $this->validate($value, $constraint); } - function it_should_add_violation_if_selected_shipping_method_is_inpost_and_payment_method_is_cash_on_deliver( + public function it_should_add_violation_if_selected_shipping_method_is_inpost_and_payment_method_is_cash_on_deliver( ExecutionContextInterface $context, Constraint $constraint, OrderInterface $value, ConstraintViolationBuilderInterface $violationBuilder, ShippingMethodCheckerInterface $shippingMethodChecker - ): void - { + ): void { $paymentMethod = PaymentMethodBuilder::create()->withCode('cash_on_delivery')->build(); $payment = PaymentBuilder::create()->withPaymentMethod($paymentMethod)->build(); diff --git a/spec/Validator/HasPhoneNumberInPostOrderValidatorSpec.php b/spec/Validator/HasPhoneNumberInPostOrderValidatorSpec.php index 95880f2..595bd52 100644 --- a/spec/Validator/HasPhoneNumberInPostOrderValidatorSpec.php +++ b/spec/Validator/HasPhoneNumberInPostOrderValidatorSpec.php @@ -14,7 +14,6 @@ use BitBag\SyliusInPostPlugin\Model\InPostPointsAwareInterface; use BitBag\SyliusInPostPlugin\Validator\Constraint\HasPhoneNumberInPostOrder; use BitBag\SyliusInPostPlugin\Validator\HasPhoneNumberInPostOrderValidator; -use Doctrine\Common\Collections\ArrayCollection; use PhpSpec\ObjectBehavior; use Prophecy\Argument; use stdClass; @@ -23,22 +22,20 @@ use Symfony\Component\Validator\Context\ExecutionContextInterface; use Symfony\Component\Validator\Violation\ConstraintViolationBuilderInterface; use Tests\BitBag\SyliusInPostPlugin\Spec\Builder\AddressBuilder; -use Tests\BitBag\SyliusInPostPlugin\Spec\ObjectMother\AddressObjectMother; -use Tests\BitBag\SyliusInPostPlugin\Spec\ObjectMother\ShipmentObjectMother; class HasPhoneNumberInPostOrderValidatorSpec extends ObjectBehavior { - function let(ShippingMethodCheckerInterface $shippingMethodChecker): void + public function let(ShippingMethodCheckerInterface $shippingMethodChecker): void { $this->beConstructedWith($shippingMethodChecker); } - function it_is_initializable() + public function it_is_initializable() { $this->shouldHaveType(HasPhoneNumberInPostOrderValidator::class); } - function it_should_do_nothing_if_value_is_not_instance_of_order_interface( + public function it_should_do_nothing_if_value_is_not_instance_of_order_interface( ExecutionContextInterface $context, Constraint $constraint ): void { @@ -49,7 +46,7 @@ function it_should_do_nothing_if_value_is_not_instance_of_order_interface( $this->validate($value, $constraint); } - function it_should_do_nothing_if_value_is_not_implementing_inpost_points_aware_interface( + public function it_should_do_nothing_if_value_is_not_implementing_inpost_points_aware_interface( ExecutionContextInterface $context, Constraint $constraint, OrderInterface $value @@ -60,7 +57,7 @@ function it_should_do_nothing_if_value_is_not_implementing_inpost_points_aware_i $this->validate($value, $constraint); } - function it_should_do_nothing_if_selected_shipping_method_is_not_inpost( + public function it_should_do_nothing_if_selected_shipping_method_is_not_inpost( ExecutionContextInterface $context, Constraint $constraint, OrderInterface $value, @@ -74,7 +71,7 @@ function it_should_do_nothing_if_selected_shipping_method_is_not_inpost( $this->validate($value, $constraint); } - function it_should_do_nothing_if_phone_number_is_set( + public function it_should_do_nothing_if_phone_number_is_set( ExecutionContextInterface $context, Constraint $constraint, OrderInterface $value, @@ -91,7 +88,7 @@ function it_should_do_nothing_if_phone_number_is_set( $this->validate($value, $constraint); } - function it_should_add_violation_if_phone_number_is_not_set( + public function it_should_add_violation_if_phone_number_is_not_set( ExecutionContextInterface $context, Constraint $constraint, OrderInterface $value, diff --git a/src/EventListener/ShippingExportEventListener.php b/src/EventListener/ShippingExportEventListener.php index 090fef8..090c990 100644 --- a/src/EventListener/ShippingExportEventListener.php +++ b/src/EventListener/ShippingExportEventListener.php @@ -74,7 +74,7 @@ public function exportShipment(ResourceControllerEvent $exportShipmentEvent): vo try { $createShipmentResponse = $this->webClient->createShipment($shipment); } catch (ClientException $exception) { - $this->requestStack->getSession()->getFlashBag()->add('error', 'bitbag.ui.shipping_export_error'); + $this->requestStack->getSession()->getBag('flashes')->add('error', 'bitbag.ui.shipping_export_error'); $this->logError($exception, $shipment); return; @@ -86,7 +86,7 @@ public function exportShipment(ResourceControllerEvent $exportShipmentEvent): vo try { $shipmentData = $this->webClient->getShipmentById((int) ($shippingExport->getExternalId())); } catch (ClientException $exception) { - $this->requestStack->getSession()->getFlashBag()->add('error', 'bitbag.ui.shipping_export_error'); + $this->requestStack->getSession()->getBag('flashes')->add('error', 'bitbag.ui.shipping_export_error'); $this->logError($exception, $shipment); return; @@ -100,7 +100,7 @@ public function exportShipment(ResourceControllerEvent $exportShipmentEvent): vo try { $action = $this->shippingExportActionProvider->provide($status); } catch (\Exception $exception) { - $this->requestStack->getSession()->getFlashBag()->add('error', 'bitbag.ui.shipping_export_error'); + $this->requestStack->getSession()->getBag('flashes')->add('error', 'bitbag.ui.shipping_export_error'); $this->logError($exception, $shipment); return; diff --git a/src/EventListener/ShippingExportEventListener/InPostShippingExportConfirmedAction.php b/src/EventListener/ShippingExportEventListener/InPostShippingExportConfirmedAction.php index c9888a3..44d286a 100644 --- a/src/EventListener/ShippingExportEventListener/InPostShippingExportConfirmedAction.php +++ b/src/EventListener/ShippingExportEventListener/InPostShippingExportConfirmedAction.php @@ -72,7 +72,7 @@ public function execute(ShippingExportInterface $shippingExport): void $shippingExport->setExportedAt(new \DateTime()); $this->shippingExportRepository->add($shippingExport); - $this->requestStack->getSession()->getFlashBag()->add(self::SUCCESS, self::TRANSLATION_KEY); + $this->requestStack->getSession()->getBag('flashes')->add(self::SUCCESS, self::TRANSLATION_KEY); } private function validateShippingExport(ShippingExportInterface $shippingExport): void diff --git a/src/EventListener/ShippingExportEventListener/InPostShippingExportDefaultAction.php b/src/EventListener/ShippingExportEventListener/InPostShippingExportDefaultAction.php index bd51fc1..933b77a 100644 --- a/src/EventListener/ShippingExportEventListener/InPostShippingExportDefaultAction.php +++ b/src/EventListener/ShippingExportEventListener/InPostShippingExportDefaultAction.php @@ -37,7 +37,7 @@ public function execute(ShippingExportInterface $shippingExport): void $this->shippingExportRepository->add($shippingExport); - $this->requestStack->getSession()->getFlashBag()->add(self::INFO, self::TRANSLATION_KEY); + $this->requestStack->getSession()->getBag('flashes')->add(self::INFO, self::TRANSLATION_KEY); } public function supports(string $statusCode): bool From a816b5e791077c7d45ff3505d6f491c936785bd8 Mon Sep 17 00:00:00 2001 From: Szymon Kostrubiec Date: Thu, 9 May 2024 13:12:34 +0200 Subject: [PATCH 03/13] op-264 Add validator and run ECS --- ...nPostShippingExportConfirmedActionSpec.php | 4 +- .../InPostShippingExportDefaultActionSpec.php | 4 +- ...lidPhoneNumberInPostOrderValidatorSpec.php | 154 ++++++++++++++++++ src/Resources/config/services/validator.xml | 12 ++ src/Resources/config/validation/Order.xml | 6 + src/Resources/translations/validators.en.yml | 3 + .../HasValidPhoneNumberInPostOrder.php | 32 ++++ ...asValidPhoneNumberInPostOrderValidator.php | 69 ++++++++ 8 files changed, 280 insertions(+), 4 deletions(-) create mode 100644 spec/Validator/HasValidPhoneNumberInPostOrderValidatorSpec.php create mode 100644 src/Validator/Constraint/HasValidPhoneNumberInPostOrder.php create mode 100644 src/Validator/HasValidPhoneNumberInPostOrderValidator.php diff --git a/spec/EventListener/ShippingExportEventListener/InPostShippingExportConfirmedActionSpec.php b/spec/EventListener/ShippingExportEventListener/InPostShippingExportConfirmedActionSpec.php index 16121f1..bc3f537 100644 --- a/spec/EventListener/ShippingExportEventListener/InPostShippingExportConfirmedActionSpec.php +++ b/spec/EventListener/ShippingExportEventListener/InPostShippingExportConfirmedActionSpec.php @@ -117,7 +117,7 @@ public function it_should_save_label_under_the_correct_path( RequestStack $requestStack, SessionInterface $session, FlashBagInterface $flashBag - ): void { + ): void { $order = OrderBuilder::create()->withNumber(self::ORDER_NUMBER)->build(); $shipment->getOrder()->willReturn($order); $shipment->getId()->willReturn(self::SHIPMENT_ID); @@ -144,7 +144,7 @@ public function it_should_update_shipping_export_entity( RequestStack $requestStack, SessionInterface $session, FlashBagInterface $flashBag - ): void { + ): void { $webClient->getLabels([self::FOO])->willReturn(self::SHIPPING_LABEL_CONTENT); $order = OrderBuilder::create()->withNumber(self::ORDER_NUMBER)->build(); $shipment->getOrder()->willReturn($order); diff --git a/spec/EventListener/ShippingExportEventListener/InPostShippingExportDefaultActionSpec.php b/spec/EventListener/ShippingExportEventListener/InPostShippingExportDefaultActionSpec.php index 462046f..59a6e96 100644 --- a/spec/EventListener/ShippingExportEventListener/InPostShippingExportDefaultActionSpec.php +++ b/spec/EventListener/ShippingExportEventListener/InPostShippingExportDefaultActionSpec.php @@ -40,7 +40,7 @@ public function it_should_set_state_and_exported_at_properties( RequestStack $requestStack, SessionInterface $session, FlashBagInterface $flashBag - ): void { + ): void { $shippingExport->setState(ShippingExportInterface::STATE_PENDING)->shouldBeCalled(); $shippingExport->setExportedAt(Argument::type(\DateTime::class))->shouldBeCalled(); @@ -56,7 +56,7 @@ public function it_should_save_shipping_export_changes( RequestStack $requestStack, SessionInterface $session, FlashBagInterface $flashBag - ): void { + ): void { $shippingExportRepository->add($shippingExport)->shouldBeCalled(); $requestStack->getSession()->willReturn($session); diff --git a/spec/Validator/HasValidPhoneNumberInPostOrderValidatorSpec.php b/spec/Validator/HasValidPhoneNumberInPostOrderValidatorSpec.php new file mode 100644 index 0000000..89aeebb --- /dev/null +++ b/spec/Validator/HasValidPhoneNumberInPostOrderValidatorSpec.php @@ -0,0 +1,154 @@ +beConstructedWith($shippingMethodChecker); + } + + public function it_is_initializable() + { + $this->shouldHaveType(HasValidPhoneNumberInPostOrderValidator::class); + } + + public function it_should_do_nothing_if_value_is_not_instance_of_order_interface( + ExecutionContextInterface $context, + Constraint $constraint + ): void { + $context->buildViolation(Argument::type('string'))->shouldNotBeCalled(); + $value = new stdClass(); + + $this->initialize($context); + $this->validate($value, $constraint); + } + + public function it_should_do_nothing_if_value_is_not_implementing_inpost_points_aware_interface( + ExecutionContextInterface $context, + Constraint $constraint, + OrderInterface $value + ): void { + $context->buildViolation(Argument::type('string'))->shouldNotBeCalled(); + + $this->initialize($context); + $this->validate($value, $constraint); + } + + public function it_should_do_nothing_if_selected_shipping_method_is_not_inpost( + ExecutionContextInterface $context, + Constraint $constraint, + OrderInterface $value, + ShippingMethodCheckerInterface $shippingMethodChecker + ): void { + $value->implement(InPostPointsAwareInterface::class); + $context->buildViolation(Argument::type('string'))->shouldNotBeCalled(); + $shippingMethodChecker->isInPost(Argument::type(OrderInterface::class))->willReturn(false); + + $this->initialize($context); + $this->validate($value, $constraint); + } + + public function it_should_do_nothing_if_phone_number_is_not_set( + ExecutionContextInterface $context, + Constraint $constraint, + OrderInterface $value, + ShippingMethodCheckerInterface $shippingMethodChecker + ): void { + $value->implement(InPostPointsAwareInterface::class); + $value->getShippingAddress()->willReturn(null); + $shippingMethodChecker->isInPost(Argument::type(OrderInterface::class))->willReturn(false); + + $context->buildViolation(Argument::type('string'))->shouldNotBeCalled(); + + $this->initialize($context); + $this->validate($value, $constraint); + } + + public function it_should_add_violation_if_phone_number_is_too_short( + ExecutionContextInterface $context, + Constraint $constraint, + OrderInterface $value, + ConstraintViolationBuilderInterface $violationBuilder, + ShippingMethodCheckerInterface $shippingMethodChecker, + HasValidPhoneNumberInPostOrder $hasValidPhoneNumberInPostOrder, + ): void { + $addressBuilder = AddressBuilder::create()->withPhoneNumber('123')->build(); + $value->implement(InPostPointsAwareInterface::class); + $value->getShippingAddress()->willReturn($addressBuilder); + $shippingMethodChecker->isInPost(Argument::type(OrderInterface::class))->willReturn(true); + + $context->buildViolation(HasValidPhoneNumberInPostOrder::PHONE_NUMBER_IS_TOO_SHORT_MESSAGE) + ->willReturn($violationBuilder); + $violationBuilder->setParameter('{{ limit }}', HasValidPhoneNumberInPostOrder::POLISH_PHONE_NUMBER_DEFAULT_LENGTH) + ->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( + ExecutionContextInterface $context, + Constraint $constraint, + OrderInterface $value, + ConstraintViolationBuilderInterface $violationBuilder, + ShippingMethodCheckerInterface $shippingMethodChecker, + HasValidPhoneNumberInPostOrder $hasValidPhoneNumberInPostOrder, + ): void { + $addressBuilder = AddressBuilder::create()->withPhoneNumber('1234567890')->build(); + $value->implement(InPostPointsAwareInterface::class); + $value->getShippingAddress()->willReturn($addressBuilder); + $shippingMethodChecker->isInPost(Argument::type(OrderInterface::class))->willReturn(true); + + $context->buildViolation(HasValidPhoneNumberInPostOrder::PHONE_NUMBER_IS_TOO_LONG_MESSAGE) + ->willReturn($violationBuilder); + $violationBuilder->setParameter('{{ limit }}', HasValidPhoneNumberInPostOrder::POLISH_PHONE_NUMBER_DEFAULT_LENGTH) + ->willReturn($violationBuilder); + $violationBuilder->atPath('shippingAddress.phoneNumber') + ->willReturn($violationBuilder); + + $violationBuilder->addViolation()->shouldBeCalled(); + + $this->initialize($context); + $this->validate($value, $constraint); + } + + public function it_should_do_nothing_if_phone_number_is_valid( + ExecutionContextInterface $context, + Constraint $constraint, + OrderInterface $value, + ConstraintViolationBuilderInterface $violationBuilder, + ShippingMethodCheckerInterface $shippingMethodChecker, + HasValidPhoneNumberInPostOrder $hasValidPhoneNumberInPostOrder, + ): void { + $addressBuilder = AddressBuilder::create()->withPhoneNumber('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/config/services/validator.xml b/src/Resources/config/services/validator.xml index fce6cd9..0c29eb4 100644 --- a/src/Resources/config/services/validator.xml +++ b/src/Resources/config/services/validator.xml @@ -25,5 +25,17 @@ alias="bitbag_sylius_inpost_plugin_validator_has_allowed_payment_method_inpost_order" /> + + + + + + diff --git a/src/Resources/config/validation/Order.xml b/src/Resources/config/validation/Order.xml index 41f4a9f..6ba7ecc 100644 --- a/src/Resources/config/validation/Order.xml +++ b/src/Resources/config/validation/Order.xml @@ -14,5 +14,11 @@ bitbag_select_shipping + + + diff --git a/src/Resources/translations/validators.en.yml b/src/Resources/translations/validators.en.yml index c49724b..80bde99 100644 --- a/src/Resources/translations/validators.en.yml +++ b/src/Resources/translations/validators.en.yml @@ -2,3 +2,6 @@ bitbag_sylius_inpost_plugin: order: 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' diff --git a/src/Validator/Constraint/HasValidPhoneNumberInPostOrder.php b/src/Validator/Constraint/HasValidPhoneNumberInPostOrder.php new file mode 100644 index 0000000..84c7e4d --- /dev/null +++ b/src/Validator/Constraint/HasValidPhoneNumberInPostOrder.php @@ -0,0 +1,32 @@ +shippingMethodChecker = $shippingMethodChecker; + } + + /** + * @param mixed $value + */ + public function validate($value, Constraint $constraint): void + { + if (!$value instanceof OrderInterface) { + return; + } + + if (!$value instanceof InPostPointsAwareInterface) { + return; + } + + $isInPostShipment = $this->shippingMethodChecker->isInPost($value); + if (false === $isInPostShipment) { + return; + } + + $phone = $value->getShippingAddress()->getPhoneNumber(); + if (null === $phone) { + return; + } + + $length = strlen($phone); + if (HasValidPhoneNumberInPostOrder::POLISH_PHONE_NUMBER_DEFAULT_LENGTH > $length) { + $this->addPhoneNumberViolation(HasValidPhoneNumberInPostOrder::PHONE_NUMBER_IS_TOO_SHORT_MESSAGE); + } + + if (HasValidPhoneNumberInPostOrder::POLISH_PHONE_NUMBER_DEFAULT_LENGTH < $length) { + $this->addPhoneNumberViolation(HasValidPhoneNumberInPostOrder::PHONE_NUMBER_IS_TOO_LONG_MESSAGE); + } + } + + 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(); + } +} From 2abb8bf64a97edc67c29088ad3e2012248fb48fd Mon Sep 17 00:00:00 2001 From: Szymon Kostrubiec Date: Thu, 9 May 2024 15:24:33 +0200 Subject: [PATCH 04/13] op-264 Update Readme --- doc/known_issues.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/doc/known_issues.md b/doc/known_issues.md index a95e5fd..bc3df5c 100644 --- a/doc/known_issues.md +++ b/doc/known_issues.md @@ -19,4 +19,7 @@ is issue caused by circular reference between `pagerfantaDefault.css` and it's l #### Workaround #### -* Use both commands `assets:install` and `sylius:theme:assets:install` without `--symlink` option \ No newline at end of file +* Use both commands `assets:install` and `sylius:theme:assets:install` without `--symlink` option + +### Sandbox - Select inPost point ### +* When you place an order in the test environment, make sure that you select a parcel collection point from the map of the Sandbox. You can find the map of available collection points in the test environment at sandbox-manager.paczkomaty.pl. Selecting a point that doesn't exist on the test map may result in errors. From 5a18d0997cacba444b74235494829ae4c8a798f7 Mon Sep 17 00:00:00 2001 From: Szymon Kostrubiec Date: Fri, 10 May 2024 13:51:38 +0200 Subject: [PATCH 05/13] OP-264 Fixed Github action build error - behat --- composer.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 23fb096..a9e534e 100644 --- a/composer.json +++ b/composer.json @@ -50,7 +50,8 @@ "symfony/browser-kit": "4.1.8", "symfony/dom-crawler": "4.1.8", "symfony/routing": "4.1.8", - "symfony/doctrine-bridge": "4.4.16" + "symfony/doctrine-bridge": "4.4.16", + "behat/mink-selenium2-driver": ">=1.7.0" }, "config": { "sort-packages": true, From 75433d5b933880feb250ad9c4fc533c451046e0a Mon Sep 17 00:00:00 2001 From: Szymon Kostrubiec Date: Mon, 13 May 2024 09:53:40 +0200 Subject: [PATCH 06/13] OP-264 Add PHPSpec fix after merge --- spec/Api/WebClientSpec.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spec/Api/WebClientSpec.php b/spec/Api/WebClientSpec.php index 7ec3831..d3cb8c5 100644 --- a/spec/Api/WebClientSpec.php +++ b/spec/Api/WebClientSpec.php @@ -24,9 +24,11 @@ final class WebClientSpec extends ObjectBehavior public const POINT_NAME = 'AAA666'; + public const LABEL_TYPE = "normal"; + public function let(Client $client): void { - $this->beConstructedWith($client); + $this->beConstructedWith($client,self::LABEL_TYPE); } public function it_is_initializable(): void From c942ab74c690cc5c41022e518505939ace0ddceb Mon Sep 17 00:00:00 2001 From: Szymon Kostrubiec Date: Wed, 22 May 2024 12:57:06 +0200 Subject: [PATCH 07/13] OP-264 - Codereview fixes added, fix tests, validator, docs --- doc/known_issues.md | 2 +- ...lidPhoneNumberInPostOrderValidatorSpec.php | 25 +++++++++++-------- src/Resources/config/services/validator.xml | 2 +- .../HasValidPhoneNumberInPostOrder.php | 2 +- ...asValidPhoneNumberInPostOrderValidator.php | 10 +++----- 5 files changed, 22 insertions(+), 19 deletions(-) diff --git a/doc/known_issues.md b/doc/known_issues.md index bc3df5c..34bbe17 100644 --- a/doc/known_issues.md +++ b/doc/known_issues.md @@ -22,4 +22,4 @@ is issue caused by circular reference between `pagerfantaDefault.css` and it's l * Use both commands `assets:install` and `sylius:theme:assets:install` without `--symlink` option ### Sandbox - Select inPost point ### -* When you place an order in the test environment, make sure that you select a parcel collection point from the map of the Sandbox. You can find the map of available collection points in the test environment at sandbox-manager.paczkomaty.pl. Selecting a point that doesn't exist on the test map may result in errors. +* When you place an order in the test environment, make sure that you select a parcel collection point from the map of the Sandbox. You can find the map of available collection points in the test environment at https://sandbox-manager.paczkomaty.pl. Selecting a point that doesn't exist on the test map may result in errors. diff --git a/spec/Validator/HasValidPhoneNumberInPostOrderValidatorSpec.php b/spec/Validator/HasValidPhoneNumberInPostOrderValidatorSpec.php index 89aeebb..45efe78 100644 --- a/spec/Validator/HasValidPhoneNumberInPostOrderValidatorSpec.php +++ b/spec/Validator/HasValidPhoneNumberInPostOrderValidatorSpec.php @@ -1,5 +1,11 @@ shouldHaveType(HasValidPhoneNumberInPostOrderValidator::class); } - public function it_should_do_nothing_if_value_is_not_instance_of_order_interface( + public function it_should_throw_exception_if_value_is_not_instance_of_order_interface( ExecutionContextInterface $context, Constraint $constraint ): void { @@ -37,10 +43,11 @@ public function it_should_do_nothing_if_value_is_not_instance_of_order_interface $value = new stdClass(); $this->initialize($context); - $this->validate($value, $constraint); + $this->shouldThrow(new \Exception('Value must be instance of OrderInterface')) + ->during('validate', [$value, $constraint]); } - public function it_should_do_nothing_if_value_is_not_implementing_inpost_points_aware_interface( + public function it_should_throw_exception_if_value_is_not_implementing_inpost_points_aware_interface( ExecutionContextInterface $context, Constraint $constraint, OrderInterface $value @@ -48,7 +55,8 @@ public function it_should_do_nothing_if_value_is_not_implementing_inpost_points_ $context->buildViolation(Argument::type('string'))->shouldNotBeCalled(); $this->initialize($context); - $this->validate($value, $constraint); + $this->shouldThrow(new \Exception('Value must be instance of InPostPointsAwareInterface')) + ->during('validate', [$value, $constraint]); } public function it_should_do_nothing_if_selected_shipping_method_is_not_inpost( @@ -87,7 +95,6 @@ public function it_should_add_violation_if_phone_number_is_too_short( OrderInterface $value, ConstraintViolationBuilderInterface $violationBuilder, ShippingMethodCheckerInterface $shippingMethodChecker, - HasValidPhoneNumberInPostOrder $hasValidPhoneNumberInPostOrder, ): void { $addressBuilder = AddressBuilder::create()->withPhoneNumber('123')->build(); $value->implement(InPostPointsAwareInterface::class); @@ -96,7 +103,7 @@ public function it_should_add_violation_if_phone_number_is_too_short( $context->buildViolation(HasValidPhoneNumberInPostOrder::PHONE_NUMBER_IS_TOO_SHORT_MESSAGE) ->willReturn($violationBuilder); - $violationBuilder->setParameter('{{ limit }}', HasValidPhoneNumberInPostOrder::POLISH_PHONE_NUMBER_DEFAULT_LENGTH) + $violationBuilder->setParameter('{{ limit }}', (string) HasValidPhoneNumberInPostOrder::POLISH_PHONE_NUMBER_DEFAULT_LENGTH) ->willReturn($violationBuilder); $violationBuilder->atPath('shippingAddress.phoneNumber') ->willReturn($violationBuilder); @@ -113,7 +120,6 @@ public function it_should_add_violation_if_phone_number_is_too_long( OrderInterface $value, ConstraintViolationBuilderInterface $violationBuilder, ShippingMethodCheckerInterface $shippingMethodChecker, - HasValidPhoneNumberInPostOrder $hasValidPhoneNumberInPostOrder, ): void { $addressBuilder = AddressBuilder::create()->withPhoneNumber('1234567890')->build(); $value->implement(InPostPointsAwareInterface::class); @@ -122,7 +128,7 @@ public function it_should_add_violation_if_phone_number_is_too_long( $context->buildViolation(HasValidPhoneNumberInPostOrder::PHONE_NUMBER_IS_TOO_LONG_MESSAGE) ->willReturn($violationBuilder); - $violationBuilder->setParameter('{{ limit }}', HasValidPhoneNumberInPostOrder::POLISH_PHONE_NUMBER_DEFAULT_LENGTH) + $violationBuilder->setParameter('{{ limit }}', (string) HasValidPhoneNumberInPostOrder::POLISH_PHONE_NUMBER_DEFAULT_LENGTH) ->willReturn($violationBuilder); $violationBuilder->atPath('shippingAddress.phoneNumber') ->willReturn($violationBuilder); @@ -139,7 +145,6 @@ public function it_should_do_nothing_if_phone_number_is_valid( OrderInterface $value, ConstraintViolationBuilderInterface $violationBuilder, ShippingMethodCheckerInterface $shippingMethodChecker, - HasValidPhoneNumberInPostOrder $hasValidPhoneNumberInPostOrder, ): void { $addressBuilder = AddressBuilder::create()->withPhoneNumber('123456789')->build(); $value->implement(InPostPointsAwareInterface::class); diff --git a/src/Resources/config/services/validator.xml b/src/Resources/config/services/validator.xml index 0c29eb4..508a883 100644 --- a/src/Resources/config/services/validator.xml +++ b/src/Resources/config/services/validator.xml @@ -28,7 +28,7 @@ diff --git a/src/Validator/Constraint/HasValidPhoneNumberInPostOrder.php b/src/Validator/Constraint/HasValidPhoneNumberInPostOrder.php index 84c7e4d..ac68f1a 100644 --- a/src/Validator/Constraint/HasValidPhoneNumberInPostOrder.php +++ b/src/Validator/Constraint/HasValidPhoneNumberInPostOrder.php @@ -12,7 +12,7 @@ use Symfony\Component\Validator\Constraint; -class HasValidPhoneNumberInPostOrder extends Constraint +final class HasValidPhoneNumberInPostOrder extends Constraint { public const PHONE_NUMBER_IS_TOO_SHORT_MESSAGE = 'bitbag_sylius_inpost_plugin.order.phone_number.min'; diff --git a/src/Validator/HasValidPhoneNumberInPostOrderValidator.php b/src/Validator/HasValidPhoneNumberInPostOrderValidator.php index f2db29b..2401253 100644 --- a/src/Validator/HasValidPhoneNumberInPostOrderValidator.php +++ b/src/Validator/HasValidPhoneNumberInPostOrderValidator.php @@ -17,7 +17,7 @@ use Symfony\Component\Validator\Constraint; use Symfony\Component\Validator\ConstraintValidator; -class HasValidPhoneNumberInPostOrderValidator extends ConstraintValidator +final class HasValidPhoneNumberInPostOrderValidator extends ConstraintValidator { private ShippingMethodCheckerInterface $shippingMethodChecker; @@ -26,17 +26,15 @@ public function __construct(ShippingMethodCheckerInterface $shippingMethodChecke $this->shippingMethodChecker = $shippingMethodChecker; } - /** - * @param mixed $value - */ + /* @param mixed $value */ public function validate($value, Constraint $constraint): void { if (!$value instanceof OrderInterface) { - return; + throw new \Exception("Value must be instance of OrderInterface"); } if (!$value instanceof InPostPointsAwareInterface) { - return; + throw new \Exception("Value must be instance of InPostPointsAwareInterface"); } $isInPostShipment = $this->shippingMethodChecker->isInPost($value); From 344191f70716740ceda8f3864ddcf464ef7d95c2 Mon Sep 17 00:00:00 2001 From: Szymon Kostrubiec Date: Wed, 22 May 2024 13:05:17 +0200 Subject: [PATCH 08/13] OP-264 - Add ECS fix for validator --- src/Validator/HasValidPhoneNumberInPostOrderValidator.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Validator/HasValidPhoneNumberInPostOrderValidator.php b/src/Validator/HasValidPhoneNumberInPostOrderValidator.php index 2401253..22db280 100644 --- a/src/Validator/HasValidPhoneNumberInPostOrderValidator.php +++ b/src/Validator/HasValidPhoneNumberInPostOrderValidator.php @@ -30,11 +30,11 @@ public function __construct(ShippingMethodCheckerInterface $shippingMethodChecke public function validate($value, Constraint $constraint): void { if (!$value instanceof OrderInterface) { - throw new \Exception("Value must be instance of OrderInterface"); + throw new \Exception('Value must be instance of OrderInterface'); } if (!$value instanceof InPostPointsAwareInterface) { - throw new \Exception("Value must be instance of InPostPointsAwareInterface"); + throw new \Exception('Value must be instance of InPostPointsAwareInterface'); } $isInPostShipment = $this->shippingMethodChecker->isInPost($value); From 0b7662eb2e87c8d416b1f21066041888b11bb8a3 Mon Sep 17 00:00:00 2001 From: Szymon Kostrubiec Date: Fri, 24 May 2024 07:40:18 +0200 Subject: [PATCH 09/13] OP-264 - Assert added for validation --- .../HasValidPhoneNumberInPostOrderValidatorSpec.php | 4 ++-- .../HasValidPhoneNumberInPostOrderValidator.php | 9 +++------ 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/spec/Validator/HasValidPhoneNumberInPostOrderValidatorSpec.php b/spec/Validator/HasValidPhoneNumberInPostOrderValidatorSpec.php index 45efe78..60404a8 100644 --- a/spec/Validator/HasValidPhoneNumberInPostOrderValidatorSpec.php +++ b/spec/Validator/HasValidPhoneNumberInPostOrderValidatorSpec.php @@ -43,7 +43,7 @@ public function it_should_throw_exception_if_value_is_not_instance_of_order_inte $value = new stdClass(); $this->initialize($context); - $this->shouldThrow(new \Exception('Value must be instance of OrderInterface')) + $this->shouldThrow(\Exception::class) ->during('validate', [$value, $constraint]); } @@ -55,7 +55,7 @@ public function it_should_throw_exception_if_value_is_not_implementing_inpost_po $context->buildViolation(Argument::type('string'))->shouldNotBeCalled(); $this->initialize($context); - $this->shouldThrow(new \Exception('Value must be instance of InPostPointsAwareInterface')) + $this->shouldThrow(\Exception::class) ->during('validate', [$value, $constraint]); } diff --git a/src/Validator/HasValidPhoneNumberInPostOrderValidator.php b/src/Validator/HasValidPhoneNumberInPostOrderValidator.php index 22db280..3c08919 100644 --- a/src/Validator/HasValidPhoneNumberInPostOrderValidator.php +++ b/src/Validator/HasValidPhoneNumberInPostOrderValidator.php @@ -16,6 +16,7 @@ use Sylius\Component\Core\Model\OrderInterface; use Symfony\Component\Validator\Constraint; use Symfony\Component\Validator\ConstraintValidator; +use Webmozart\Assert\Assert; final class HasValidPhoneNumberInPostOrderValidator extends ConstraintValidator { @@ -29,13 +30,9 @@ public function __construct(ShippingMethodCheckerInterface $shippingMethodChecke /* @param mixed $value */ public function validate($value, Constraint $constraint): void { - if (!$value instanceof OrderInterface) { - throw new \Exception('Value must be instance of OrderInterface'); - } + Assert::isInstanceOf($value, OrderInterface::class); - if (!$value instanceof InPostPointsAwareInterface) { - throw new \Exception('Value must be instance of InPostPointsAwareInterface'); - } + Assert::isInstanceOf($value, InPostPointsAwareInterface::class); $isInPostShipment = $this->shippingMethodChecker->isInPost($value); if (false === $isInPostShipment) { From 31b65ce37f71ddb145df0211fb0726c78f4c7daf Mon Sep 17 00:00:00 2001 From: Szymon Kostrubiec Date: Mon, 3 Jun 2024 13:01:10 +0200 Subject: [PATCH 10/13] OP-264 - Fix for validation, added support for Polish prefixes, removed min/max phone validation message --- ...lidPhoneNumberInPostOrderValidatorSpec.php | 194 +++++++++++++++++- src/Resources/translations/validators.en.yml | 3 +- .../HasValidPhoneNumberInPostOrder.php | 13 +- ...asValidPhoneNumberInPostOrderValidator.php | 25 ++- 4 files changed, 215 insertions(+), 20 deletions(-) diff --git a/spec/Validator/HasValidPhoneNumberInPostOrderValidatorSpec.php b/spec/Validator/HasValidPhoneNumberInPostOrderValidatorSpec.php index 60404a8..a72eeed 100644 --- 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 index 80bde99..4ac391c 100644 --- 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 index ac68f1a..693fa4e 100644 --- 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 index 3c08919..c9d4d59 100644 --- 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; + } } From 58de671299ad4c80e6d2f691e25e22c57ae9525b Mon Sep 17 00:00:00 2001 From: Szymon Kostrubiec Date: Tue, 4 Jun 2024 07:23:38 +0200 Subject: [PATCH 11/13] OP-264 - Fix for constants and methods, removed polish from names --- ...lidPhoneNumberInPostOrderValidatorSpec.php | 24 +++++++++---------- .../HasValidPhoneNumberInPostOrder.php | 4 ++-- ...asValidPhoneNumberInPostOrderValidator.php | 4 ++-- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/spec/Validator/HasValidPhoneNumberInPostOrderValidatorSpec.php b/spec/Validator/HasValidPhoneNumberInPostOrderValidatorSpec.php index a72eeed..7c807f8 100644 --- 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_without_polish_prefix( + public function it_should_add_violation_if_phone_number_is_too_short_without_prefix( ExecutionContextInterface $context, Constraint $constraint, OrderInterface $value, @@ -112,7 +112,7 @@ public function it_should_add_violation_if_phone_number_is_too_short_without_pol $this->validate($value, $constraint); } - public function it_should_add_violation_if_phone_number_is_too_short_with_polish_prefix( + public function it_should_add_violation_if_phone_number_is_too_short_with_prefix( ExecutionContextInterface $context, Constraint $constraint, OrderInterface $value, @@ -135,7 +135,7 @@ public function it_should_add_violation_if_phone_number_is_too_short_with_polish $this->validate($value, $constraint); } - public function it_should_add_violation_if_phone_number_is_too_short_with_polish_long_prefix( + public function it_should_add_violation_if_phone_number_is_too_short_with_long_prefix( ExecutionContextInterface $context, Constraint $constraint, OrderInterface $value, @@ -158,7 +158,7 @@ public function it_should_add_violation_if_phone_number_is_too_short_with_polish $this->validate($value, $constraint); } - public function it_should_add_violation_if_phone_number_is_too_long_without_polish_prefix( + public function it_should_add_violation_if_phone_number_is_too_long_without_prefix( ExecutionContextInterface $context, Constraint $constraint, OrderInterface $value, @@ -181,7 +181,7 @@ public function it_should_add_violation_if_phone_number_is_too_long_without_poli $this->validate($value, $constraint); } - public function it_should_add_violation_if_phone_number_is_too_long_with_polish_prefix( + public function it_should_add_violation_if_phone_number_is_too_long_with_prefix( ExecutionContextInterface $context, Constraint $constraint, OrderInterface $value, @@ -204,7 +204,7 @@ public function it_should_add_violation_if_phone_number_is_too_long_with_polish_ $this->validate($value, $constraint); } - public function it_should_add_violation_if_phone_number_is_too_long_with_polish_long_prefix( + public function it_should_add_violation_if_phone_number_is_too_long_with_long_prefix( ExecutionContextInterface $context, Constraint $constraint, OrderInterface $value, @@ -227,7 +227,7 @@ public function it_should_add_violation_if_phone_number_is_too_long_with_polish_ $this->validate($value, $constraint); } - public function it_should_do_nothing_if_phone_number_is_valid_without_polish_prefix( + public function it_should_do_nothing_if_phone_number_is_valid_without_prefix( ExecutionContextInterface $context, Constraint $constraint, OrderInterface $value, @@ -245,7 +245,7 @@ public function it_should_do_nothing_if_phone_number_is_valid_without_polish_pre $this->validate($value, $constraint); } - public function it_should_do_nothing_if_phone_number_is_valid_with_polish_long_prefix( + public function it_should_do_nothing_if_phone_number_is_valid_with_long_prefix( ExecutionContextInterface $context, Constraint $constraint, OrderInterface $value, @@ -263,7 +263,7 @@ public function it_should_do_nothing_if_phone_number_is_valid_with_polish_long_p $this->validate($value, $constraint); } - public function it_should_do_nothing_if_phone_number_is_valid_with_polish_long_prefix_without_plus( + public function it_should_do_nothing_if_phone_number_is_valid_with_long_prefix_without_plus( ExecutionContextInterface $context, Constraint $constraint, OrderInterface $value, @@ -281,7 +281,7 @@ public function it_should_do_nothing_if_phone_number_is_valid_with_polish_long_p $this->validate($value, $constraint); } - public function it_should_do_nothing_if_phone_number_is_valid_with_polish_prefix_without_plus( + public function it_should_do_nothing_if_phone_number_is_valid_with_prefix_without_plus( ExecutionContextInterface $context, Constraint $constraint, OrderInterface $value, @@ -299,7 +299,7 @@ public function it_should_do_nothing_if_phone_number_is_valid_with_polish_prefix $this->validate($value, $constraint); } - public function it_should_do_nothing_if_phone_number_is_valid_with_polish_prefix_without_plus_with_dot( + public function it_should_do_nothing_if_phone_number_is_valid_with_prefix_without_plus_with_dot( ExecutionContextInterface $context, Constraint $constraint, OrderInterface $value, @@ -317,7 +317,7 @@ public function it_should_do_nothing_if_phone_number_is_valid_with_polish_prefix $this->validate($value, $constraint); } - public function it_should_do_nothing_if_phone_number_is_valid_with_polish_prefix_with_plus_and_dot( + public function it_should_do_nothing_if_phone_number_is_valid_with_prefix_with_plus_and_dot( ExecutionContextInterface $context, Constraint $constraint, OrderInterface $value, diff --git a/src/Validator/Constraint/HasValidPhoneNumberInPostOrder.php b/src/Validator/Constraint/HasValidPhoneNumberInPostOrder.php index 693fa4e..7607d1f 100644 --- a/src/Validator/Constraint/HasValidPhoneNumberInPostOrder.php +++ b/src/Validator/Constraint/HasValidPhoneNumberInPostOrder.php @@ -16,9 +16,9 @@ final class HasValidPhoneNumberInPostOrder extends Constraint { 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 PHONE_NUMBER_DEFAULT_LENGTH = 9; - public const POSSIBLE_POLISH_PHONE_PREFIXES = [ + public const POSSIBLE_PHONE_PREFIXES = [ '+0048', '0048', '+48.', diff --git a/src/Validator/HasValidPhoneNumberInPostOrderValidator.php b/src/Validator/HasValidPhoneNumberInPostOrderValidator.php index c9d4d59..fd726f7 100644 --- a/src/Validator/HasValidPhoneNumberInPostOrderValidator.php +++ b/src/Validator/HasValidPhoneNumberInPostOrderValidator.php @@ -48,7 +48,7 @@ public function validate($value, Constraint $constraint): void $length = strlen($formatedPhone); - if (HasValidPhoneNumberInPostOrder::POLISH_PHONE_NUMBER_DEFAULT_LENGTH !== $length) { + if (HasValidPhoneNumberInPostOrder::PHONE_NUMBER_DEFAULT_LENGTH !== $length) { $this->addPhoneNumberViolation(HasValidPhoneNumberInPostOrder::PHONE_NUMBER_LENGTH_INCORRECT); } } @@ -65,7 +65,7 @@ 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); + $escaped_prefixes = array_map('preg_quote', HasValidPhoneNumberInPostOrder::POSSIBLE_PHONE_PREFIXES); $pattern = '/^(' . implode('|', $escaped_prefixes) . ')/'; $phone = preg_replace($pattern, '', $phone); } From a48da1b4305d71a80593f9dca10888e9a3512190 Mon Sep 17 00:00:00 2001 From: Szymon Kostrubiec Date: Tue, 4 Jun 2024 07:30:08 +0200 Subject: [PATCH 12/13] OP-264 - Fix translations --- src/Resources/translations/validators.en.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Resources/translations/validators.en.yml b/src/Resources/translations/validators.en.yml index 4ac391c..300cf75 100644 --- a/src/Resources/translations/validators.en.yml +++ b/src/Resources/translations/validators.en.yml @@ -3,4 +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: - incorrect_length: "Incorrect phone number for Polish shipping methods" + incorrect_length: "Incorrect phone number, phone number must be 9 digits." From b001825dd3fe406f4211e81f4868d7ff3f82fc8f Mon Sep 17 00:00:00 2001 From: Szymon Kostrubiec Date: Tue, 4 Jun 2024 07:33:03 +0200 Subject: [PATCH 13/13] OP-264 - Added missing method parameter --- src/Validator/HasValidPhoneNumberInPostOrderValidator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Validator/HasValidPhoneNumberInPostOrderValidator.php b/src/Validator/HasValidPhoneNumberInPostOrderValidator.php index fd726f7..50ec902 100644 --- a/src/Validator/HasValidPhoneNumberInPostOrderValidator.php +++ b/src/Validator/HasValidPhoneNumberInPostOrderValidator.php @@ -60,7 +60,7 @@ private function addPhoneNumberViolation(string $message): void ->addViolation(); } - private function formatPhoneNumber($phone): string + private function formatPhoneNumber(string $phone): string { $phone = preg_replace('/\s+/', '', $phone);