PAYOSWXP-158: Improve address validation

PAYONE-GmbH · Dec 18, 2024 · 7bd5375 · 7bd5375
1 parent 9a668e1
commit 7bd5375
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 62 deletions.
diff --git a/src/Components/GenericExpressCheckout/CustomerRegistrationUtil.php b/src/Components/GenericExpressCheckout/CustomerRegistrationUtil.php
@@ -13,9 +13,12 @@
 use Shopware\Core\Framework\DataAbstractionLayer\Search\Criteria;
 use Shopware\Core\Framework\DataAbstractionLayer\Search\Filter\EqualsFilter;
 use Shopware\Core\Framework\Validation\DataBag\RequestDataBag;
+use Shopware\Core\Framework\Validation\DataValidationFactoryInterface;
+use Shopware\Core\Framework\Validation\DataValidator;
 use Shopware\Core\System\Country\CountryEntity;
+use Shopware\Core\System\SalesChannel\SalesChannelContext;
 use Shopware\Core\System\Salutation\SalutationEntity;
-use Shopware\Core\System\SystemConfig\SystemConfigService;
+use Symfony\Component\Validator\ConstraintViolationList;
 use Symfony\Contracts\Translation\TranslatorInterface;
 
 class CustomerRegistrationUtil
@@ -24,14 +27,15 @@ public function __construct(
         private readonly EntityRepository $salutationRepository,
         private readonly EntityRepository $countryRepository,
         private readonly TranslatorInterface $translator,
-        private readonly SystemConfigService $systemConfigService,
+        private readonly DataValidationFactoryInterface $addressValidationFactory,
+        private readonly DataValidator $validator,
         private readonly LoggerInterface $logger
     ) {
     }
 
-    public function getCustomerDataBagFromGetCheckoutSessionResponse(array $response, Context $context): RequestDataBag
+    public function getCustomerDataBagFromGetCheckoutSessionResponse(array $response, SalesChannelContext $salesChannelContext): RequestDataBag
     {
-        $salutationId = $this->getSalutationId($context);
+        $salutationId = $this->getSalutationId($salesChannelContext->getContext());
 
         $billingAddress = [
             'salutationId' => $salutationId,
@@ -42,7 +46,7 @@ public function getCustomerDataBagFromGetCheckoutSessionResponse(array $response
             'additionalAddressLine1' => $this->extractBillingData($response, 'addressaddition'),
             'zipcode' => $this->extractBillingData($response, 'zip'),
             'city' => $this->extractBillingData($response, 'city'),
-            'countryId' => $this->getCountryIdByCode($this->extractBillingData($response, 'country') ?? '', $context),
+            'countryId' => $this->getCountryIdByCode($this->extractBillingData($response, 'country') ?? '', $salesChannelContext->getContext()),
             'phone' => $this->extractBillingData($response, 'telephonenumber'),
         ];
 
@@ -55,18 +59,22 @@ public function getCustomerDataBagFromGetCheckoutSessionResponse(array $response
             'additionalAddressLine1' => $this->extractShippingData($response, 'addressaddition'),
             'zipcode' => $this->extractShippingData($response, 'zip'),
             'city' => $this->extractShippingData($response, 'city'),
-            'countryId' => $this->getCountryIdByCode($this->extractShippingData($response, 'country') ?? '', $context),
+            'countryId' => $this->getCountryIdByCode($this->extractShippingData($response, 'country') ?? '', $salesChannelContext->getContext()),
             'phone' => $this->extractShippingData($response, 'telephonenumber'),
         ];
 
-        $isBillingAddressComplete = $this->hasAddressRequiredData($billingAddress);
-        $isShippingAddressComplete = $this->hasAddressRequiredData($shippingAddress);
+        $billingAddressViolations = $this->validateAddress($billingAddress, $salesChannelContext);
+        $shippingAddressViolations = $this->validateAddress($shippingAddress, $salesChannelContext);
+
+        $isBillingAddressComplete = $billingAddressViolations->count() === 0;
+        $isShippingAddressComplete = $shippingAddressViolations->count() === 0;
 
         if (!$isBillingAddressComplete && !$isShippingAddressComplete) {
             $this->logger->error('PAYONE Express Checkout: The delivery and billing address is incomplete', [
                 'billingAddress' => $billingAddress,
                 'shippingAddress' => $shippingAddress,
-                'requiredFields' => $this->getRequiredFields(),
+                'billingAddressViolations' => $billingAddressViolations,
+                'shippingAddressViolations' => $shippingAddressViolations,
             ]);
 
             throw new RuntimeException($this->translator->trans('PayonePayment.errorMessages.genericError'));
@@ -168,47 +176,10 @@ private function getCountryIdByCode(string $code, Context $context): ?string
         return $country->getId();
     }
 
-    private function hasAddressRequiredData(array $address): bool
-    {
-        foreach ($this->getRequiredFields() as $field) {
-            if (!isset($address[$field])) {
-                return false;
-            }
-        }
-
-        return true;
-    }
-
-    private function getRequiredFields(): array
+    private function validateAddress(array $address, SalesChannelContext $salesChannelContext): ConstraintViolationList
     {
-        $requiredFields = [
-            'firstName',
-            'lastName',
-            'city',
-            'street',
-            'countryId',
-        ];
-
-        $phoneRequired = $this->systemConfigService->get('core.loginRegistration.phoneNumberFieldRequired') ?? false;
-        if ($phoneRequired) {
-            $requiredFields[] = 'phone';
-        }
-
-        $birthdayRequired = $this->systemConfigService->get('core.loginRegistration.birthdayFieldRequired') ?? false;
-        if ($birthdayRequired) {
-            $requiredFields[] = 'birthday';
-        }
-
-        $additionalAddress1Required = $this->systemConfigService->get('core.loginRegistration.additionalAddressField1Required') ?? false;
-        if ($additionalAddress1Required) {
-            $requiredFields[] = 'additionalAddressLine1';
-        }
-
-        $additionalAddress2Required = $this->systemConfigService->get('core.loginRegistration.additionalAddressField2Required') ?? false;
-        if ($additionalAddress2Required) {
-            $requiredFields[] = 'additionalAddressLine2';
-        }
+        $validation = $this->addressValidationFactory->create($salesChannelContext);
 
-        return $requiredFields;
+        return $this->validator->getViolations($address, $validation);
     }
 }
diff --git a/src/DependencyInjection/services.xml b/src/DependencyInjection/services.xml
@@ -124,7 +124,8 @@
             <argument key="$translator" type="service" id="translator" />
             <argument key="$countryRepository" type="service" id="country.repository" />
             <argument key="$salutationRepository" type="service" id="salutation.repository" />
-            <argument key="$systemConfigService" type="service" id="Shopware\Core\System\SystemConfig\SystemConfigService" />
+            <argument key="$addressValidationFactory" type="service" id="Shopware\Core\Checkout\Customer\Validation\AddressValidationFactory"/>
+            <argument key="$validator" type="service" id="Shopware\Core\Framework\Validation\DataValidator"/>
             <argument key="$logger" type="service" id="monolog.logger.payone" />
         </service>
 

diff --git a/src/Storefront/Controller/GenericExpressController.php b/src/Storefront/Controller/GenericExpressController.php
@@ -136,7 +136,7 @@ public function returnAction(SalesChannelContext $context, string $paymentMethod
             throw new RuntimeException($this->trans('PayonePayment.errorMessages.genericError'));
         }
 
-        $customerDataBag = $this->customerRegistrationUtil->getCustomerDataBagFromGetCheckoutSessionResponse($response, $context->getContext());
+        $customerDataBag = $this->customerRegistrationUtil->getCustomerDataBagFromGetCheckoutSessionResponse($response, $context);
         $customerResponse = $this->registerRoute->register($customerDataBag, $context, false);
         $customerId = $customerResponse->getCustomer()->getId();
         $newContextToken = $this->accountService->login($response['addpaydata']['email'], $context, true);

diff --git a/tests/Components/GenericExpressCheckout/CustomerRegistrationUtilTest.php b/tests/Components/GenericExpressCheckout/CustomerRegistrationUtilTest.php
@@ -7,7 +7,6 @@
 use PayonePayment\TestCaseBase\PayoneTestBehavior;
 use PHPUnit\Framework\TestCase;
 use Shopware\Core\Checkout\Customer\CustomerEntity;
-use Shopware\Core\Framework\Context;
 use Shopware\Core\Framework\Uuid\Uuid;
 
 /**
@@ -19,6 +18,8 @@ class CustomerRegistrationUtilTest extends TestCase
 
     public function testItReturnsCorrectDataWhenBillingDataIsMissingInResponse(): void
     {
+        $salesChannelContext = $this->createSalesChannelContext();
+
         /** @var CustomerRegistrationUtil $util */
         $util = $this->getContainer()->get(CustomerRegistrationUtil::class);
 
@@ -36,7 +37,7 @@ public function testItReturnsCorrectDataWhenBillingDataIsMissingInResponse(): vo
                 'shipping_country' => 'DE',
                 'shipping_telephonenumber' => '0123456789',
             ],
-        ], Context::createDefaultContext())->all();
+        ], $salesChannelContext)->all();
 
         static::assertArrayHasKey('guest', $data);
         static::assertTrue($data['guest'], 'guest should be always `true`, because the customer did not select if he wan\'t to be a customer');
@@ -67,6 +68,8 @@ public function testItReturnsCorrectDataWhenBillingDataIsMissingInResponse(): vo
      */
     public function testItReturnsCorrectDataWhenShippingDataIsMissingInResponse(string $billingPrefix): void
     {
+        $salesChannelContext = $this->createSalesChannelContext();
+
         /** @var CustomerRegistrationUtil $util */
         $util = $this->getContainer()->get(CustomerRegistrationUtil::class);
 
@@ -84,7 +87,7 @@ public function testItReturnsCorrectDataWhenShippingDataIsMissingInResponse(stri
                 $billingPrefix . 'state' => 'Ohio',
                 $billingPrefix . 'country' => 'DE',
             ],
-        ], Context::createDefaultContext())->all();
+        ], $salesChannelContext)->all();
 
         static::assertArrayHasKey('billingAddress', $data);
         static::assertArrayNotHasKey('shippingAddress', $data, 'shipping address should not be present, because it would be the same as billing address');
@@ -106,6 +109,8 @@ public function testItReturnsCorrectDataWhenShippingDataIsMissingInResponse(stri
      */
     public function testItReturnsCorrectDataWhenDifferentShippingAndBillingAreAvailableInResponse(string $billingPrefix): void
     {
+        $salesChannelContext = $this->createSalesChannelContext();
+
         /** @var CustomerRegistrationUtil $util */
         $util = $this->getContainer()->get(CustomerRegistrationUtil::class);
 
@@ -135,7 +140,7 @@ public function testItReturnsCorrectDataWhenDifferentShippingAndBillingAreAvaila
                 'shipping_state' => 'New York',
                 'shipping_country' => 'AT',
             ],
-        ], Context::createDefaultContext())->all();
+        ], $salesChannelContext)->all();
 
         static::assertArrayHasKey('billingAddress', $data);
         static::assertArrayHasKey('shippingAddress', $data);
@@ -166,6 +171,8 @@ public function testItReturnsCorrectDataWhenDifferentShippingAndBillingAreAvaila
      */
     public function testItReturnsCorrectDataWhenSameShippingAndBillingAreAvailableInResponse(string $billingPrefix): void
     {
+        $salesChannelContext = $this->createSalesChannelContext();
+
         /** @var CustomerRegistrationUtil $util */
         $util = $this->getContainer()->get(CustomerRegistrationUtil::class);
 
@@ -195,7 +202,7 @@ public function testItReturnsCorrectDataWhenSameShippingAndBillingAreAvailableIn
                 'shipping_state' => 'Ohio',
                 'shipping_country' => 'DE',
             ],
-        ], Context::createDefaultContext())->all();
+        ], $salesChannelContext)->all();
 
         static::assertArrayHasKey('billingAddress', $data);
         static::assertArrayNotHasKey('shippingAddress', $data, 'shipping address should not be present, because it would be the same as billing address');
@@ -216,6 +223,8 @@ public function testItReturnsCorrectDataWhenSameShippingAndBillingAreAvailableIn
      */
     public function testItReturnsCorrectDataWhenBillingCompanyIsAvailableInResponse(string $billingPrefix): void
     {
+        $salesChannelContext = $this->createSalesChannelContext();
+
         /** @var CustomerRegistrationUtil $util */
         $util = $this->getContainer()->get(CustomerRegistrationUtil::class);
 
@@ -234,7 +243,7 @@ public function testItReturnsCorrectDataWhenBillingCompanyIsAvailableInResponse(
                 $billingPrefix . 'state' => 'Ohio',
                 $billingPrefix . 'country' => 'DE',
             ],
-        ], Context::createDefaultContext())->all();
+        ], $salesChannelContext)->all();
 
         static::assertArrayHasKey('billingAddress', $data);
         static::assertEquals('my-company', $data['billingAddress']['company']);
@@ -247,6 +256,8 @@ public function testItReturnsCorrectDataWhenBillingCompanyIsAvailableInResponse(
      */
     public function testItReturnsCorrectDataWhenNoBillingCompanyIsAvailableInResponse(string $billingPrefix): void
     {
+        $salesChannelContext = $this->createSalesChannelContext();
+
         /** @var CustomerRegistrationUtil $util */
         $util = $this->getContainer()->get(CustomerRegistrationUtil::class);
 
@@ -264,7 +275,7 @@ public function testItReturnsCorrectDataWhenNoBillingCompanyIsAvailableInRespons
                 $billingPrefix . 'state' => 'Ohio',
                 $billingPrefix . 'country' => 'DE',
             ],
-        ], Context::createDefaultContext())->all();
+        ], $salesChannelContext)->all();
 
         static::assertArrayHasKey('billingAddress', $data);
         static::assertNull($data['billingAddress']['company'] ?? null);
@@ -273,6 +284,8 @@ public function testItReturnsCorrectDataWhenNoBillingCompanyIsAvailableInRespons
 
     public function testItReturnsCorrectNameDataForPayPalV1(): void
     {
+        $salesChannelContext = $this->createSalesChannelContext();
+
         /** @var CustomerRegistrationUtil $util */
         $util = $this->getContainer()->get(CustomerRegistrationUtil::class);
 
@@ -300,7 +313,7 @@ public function testItReturnsCorrectNameDataForPayPalV1(): void
                 'shipping_state' => 'New York',
                 'shipping_country' => 'AT',
             ],
-        ], Context::createDefaultContext())->all();
+        ], $salesChannelContext)->all();
 
         static::assertArrayHasKey('billingAddress', $data);
         static::assertArrayHasKey('firstName', $data);
@@ -320,6 +333,8 @@ public function testItReturnsCorrectNameDataForPayPalV1(): void
      */
     public function testItThrowsExceptionIfBillingAndShippingAddressAreIncomplete(string $billingPrefix): void
     {
+        $salesChannelContext = $this->createSalesChannelContext();
+
         /** @var CustomerRegistrationUtil $util */
         $util = $this->getContainer()->get(CustomerRegistrationUtil::class);
 
@@ -349,7 +364,7 @@ public function testItThrowsExceptionIfBillingAndShippingAddressAreIncomplete(st
                 'shipping_state' => 'Ohio',
                 'shipping_country' => 'DE',
             ],
-        ], Context::createDefaultContext())->all();
+        ], $salesChannelContext)->all();
     }
 
     /**
@@ -358,6 +373,8 @@ public function testItThrowsExceptionIfBillingAndShippingAddressAreIncomplete(st
      */
     public function testItTakesCompleteShippingAddressAsBillingAddressIfBillingAddressIsIncomplete(string $billingPrefix): void
     {
+        $salesChannelContext = $this->createSalesChannelContext();
+
         /** @var CustomerRegistrationUtil $util */
         $util = $this->getContainer()->get(CustomerRegistrationUtil::class);
 
@@ -387,7 +404,7 @@ public function testItTakesCompleteShippingAddressAsBillingAddressIfBillingAddre
                 'shipping_state' => 'New York',
                 'shipping_country' => 'AT',
             ],
-        ], Context::createDefaultContext())->all();
+        ], $salesChannelContext)->all();
 
         static::assertArrayHasKey('billingAddress', $data);
         static::assertArrayNotHasKey('shippingAddress', $data, 'shipping address should not be present, because it would be the same as billing address');
@@ -404,6 +421,8 @@ public function testItTakesCompleteShippingAddressAsBillingAddressIfBillingAddre
 
     public function testItRemovesShippingAddressIfShippingAddressIsIncomplete(): void
     {
+        $salesChannelContext = $this->createSalesChannelContext();
+
         /** @var CustomerRegistrationUtil $util */
         $util = $this->getContainer()->get(CustomerRegistrationUtil::class);
 
@@ -432,7 +451,7 @@ public function testItRemovesShippingAddressIfShippingAddressIsIncomplete(): voi
                 'shipping_state' => 'New York',
                 'shipping_country' => 'AT',
             ],
-        ], Context::createDefaultContext())->all();
+        ], $salesChannelContext)->all();
 
         static::assertArrayHasKey('billingAddress', $data);
         static::assertArrayNotHasKey('shippingAddress', $data, 'shipping address should not be present, because it was incomplete');