PRE-3761: Stop requiring the phone region to match the address countr…

CHANGELOG.md

@@ -40,6 +40,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 > Merchants will need to contact support to switch to the new authentication method.
 
 ### Changed
+- Requires `payplug/unified-plugin-core` 1.2.2 or later
 - Plugin structure has been changed to follow the new Symfony bundle structure
 - Front assets have been migrated to use Stimulus
 - Gateway uniqueness is now enforced **per channel** instead of per installation: two gateways of
@@ -111,6 +112,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   error is logged
 - Hosted Fields iframes are only mounted once the payment method's form is displayed, so they no
   longer render broken when the method is preselected or chosen before its form is shown
+- A phone number valid in a country other than the address, or a metropolitan French number entered
+  on an overseas address (GP, MQ, RE, GF, YT, PM, BL, MF, NC, PF, WF), is no longer dropped from the
+  billing and shipping details sent to PayPlug (Hosted Fields), and the overseas case is also fixed
+  for the other payment methods; a local overseas number keeps its own territory
 
 ### Breaking changes for anyone extending the plugin
 

composer.json

@@ -14,7 +14,7 @@
         "ext-json": "*",
         "giggsey/libphonenumber-for-php": "^8.12",
         "payplug/payplug-php": "^4.0",
-        "payplug/unified-plugin-core": "^1.2.1",
+        "payplug/unified-plugin-core": "^1.2.2",
         "php-http/message-factory": "^1.1",
         "sylius/refund-plugin": "^2.0",
         "sylius/sylius": "^2.0",

src/Creator/PayPlugPaymentDataCreator.php

@@ -22,6 +22,7 @@
 use PayPlug\SyliusPayPlugPlugin\Gateway\ScalapayGatewayFactory;
 use PayPlug\SyliusPayPlugPlugin\Gateway\WeroGatewayFactory;
 use PayPlug\SyliusPayPlugPlugin\Upc\CustomerTitleResolver;
+use PayPlug\SyliusPayPlugPlugin\Upc\PhoneRegionResolver;
 use Sylius\Component\Core\Model\AddressInterface;
 use Sylius\Component\Core\Model\CustomerInterface;
 use Sylius\Component\Core\Model\OrderInterface;
@@ -112,31 +113,36 @@ public function create(
         return $details;
     }
 
+    /**
+     * The number is read in each region of PhoneRegionResolver::resolve() in turn, the first one
+     * in which it is valid wins.
+     */
     public function formatNumber(string $phoneNumber, ?string $isoCode): array
     {
-        try {
-            $phoneNumberUtil = PhoneNumberUtil::getInstance();
-            $parsed = $phoneNumberUtil->parse($phoneNumber, $isoCode);
+        $phoneNumberUtil = PhoneNumberUtil::getInstance();
+        $regions = null !== $isoCode ? PhoneRegionResolver::resolve($isoCode) : [null];
 
-            if (!$phoneNumberUtil->isValidNumber($parsed)) {
-                return [
-                    'phone' => null,
-                    'is_mobile' => null,
-                ];
+        foreach ($regions as $region) {
+            try {
+                $parsed = $phoneNumberUtil->parse($phoneNumber, $region);
+            } catch (NumberParseException) {
+                continue;
             }
 
-            $formatted = $phoneNumberUtil->format($parsed, PhoneNumberFormat::E164);
+            if (!$phoneNumberUtil->isValidNumber($parsed)) {
+                continue;
+            }
 
             return [
-                'phone' => $formatted,
+                'phone' => $phoneNumberUtil->format($parsed, PhoneNumberFormat::E164),
                 'is_mobile' => PhoneNumberType::MOBILE === $phoneNumberUtil->getNumberType($parsed),
             ];
-        } catch (NumberParseException) {
-            return [
-                'phone' => null,
-                'is_mobile' => null,
-            ];
         }
+
+        return [
+            'phone' => null,
+            'is_mobile' => null,
+        ];
     }
 
     private function formatTitle(CustomerInterface $customer): ?string

src/Upc/OrderAddressDtoCreator.php

@@ -22,7 +22,13 @@
  */
 final class OrderAddressDtoCreator
 {
-    public function createBilling(OrderInterface $order): ?BillingDto
+    /**
+     * @param bool $requireRegionMatch true: the phone number must belong to one of the address's
+     *      phoneRegions(); false: any valid number is sent, formatted in its own country.
+     *      Defaults to true to keep the previous behaviour for any other caller: the Hosted
+     *      Fields capture path (PaymentCaptureContextBuilder) passes false explicitly.
+     */
+    public function createBilling(OrderInterface $order, bool $requireRegionMatch = true): ?BillingDto
     {
         $address = $order->getBillingAddress();
         if (null === $address) {
@@ -31,12 +37,15 @@ public function createBilling(OrderInterface $order): ?BillingDto
 
         return new BillingDto(
             $this->buildAddress($address),
-            $this->buildContact($address),
+            $this->buildContact($address, $requireRegionMatch),
             $this->title($order),
         );
     }
 
-    public function createShipping(OrderInterface $order): ?ShippingDto
+    /**
+     * @param bool $requireRegionMatch same as createBilling()
+     */
+    public function createShipping(OrderInterface $order, bool $requireRegionMatch = true): ?ShippingDto
     {
         $address = $order->getShippingAddress();
         if (null === $address) {
@@ -45,15 +54,15 @@ public function createShipping(OrderInterface $order): ?ShippingDto
 
         return new ShippingDto(
             $this->buildAddress($address),
-            $this->buildContact($address),
+            $this->buildContact($address, $requireRegionMatch),
             $order->getCustomer()?->getEmail(),
             $address->getCompany(),
         );
     }
 
-    private function buildContact(AddressInterface $address): ContactDto
+    private function buildContact(AddressInterface $address, bool $requireRegionMatch): ContactDto
     {
-        [$phone, $mobilePhone] = $this->splitPhone($address);
+        [$phone, $mobilePhone] = $this->splitPhone($address, $requireRegionMatch);
 
         return new ContactDto($address->getFirstName(), $address->getLastName(), $phone, $mobilePhone);
     }
@@ -86,23 +95,37 @@ private function shortProvinceCode(AddressInterface $address): ?string
      *      phone number per address, so only one of the two ever comes back non-null here, matching
      *      whichever type PhoneHelper::isMobile() detects it as; an unparseable/invalid number is
      *      treated as absent rather than failing the whole payment over supplementary contact data.
+     *      The number is read in each region of phoneRegions() in turn, the first one accepting it wins.
      */
-    private function splitPhone(AddressInterface $address): array
+    private function splitPhone(AddressInterface $address, bool $requireRegionMatch): array
     {
         $rawPhone = $address->getPhoneNumber();
         $countryCode = $address->getCountryCode();
         if (null === $rawPhone || '' === $rawPhone || null === $countryCode) {
             return [null, null];
         }
 
-        try {
-            $e164Phone = PhoneHelper::toE164($rawPhone, $countryCode);
-            $isMobile = PhoneHelper::isMobile($rawPhone, $countryCode);
-        } catch (InvalidPhoneNumberException) {
-            return [null, null];
+        foreach ($this->phoneRegions($countryCode) as $region) {
+            try {
+                $e164Phone = PhoneHelper::toE164($rawPhone, $region, $requireRegionMatch);
+                $isMobile = PhoneHelper::isMobile($rawPhone, $region, $requireRegionMatch);
+            } catch (InvalidPhoneNumberException) {
+                continue;
+            }
+
+            return $isMobile ? [null, $e164Phone] : [$e164Phone, null];
         }
 
-        return $isMobile ? [null, $e164Phone] : [$e164Phone, null];
+        return [null, null];
+    }
+
+    /**
+     * @return list<string> the address country first, then the regions sharing its numbering, then
+     *      FR for an overseas territory sharing the French numbering
+     */
+    private function phoneRegions(string $countryCode): array
+    {
+        return PhoneRegionResolver::resolve($countryCode);
     }
 
     private function title(OrderInterface $order): ?string

src/Upc/PaymentCaptureContextBuilder.php

@@ -71,8 +71,10 @@ public function buildCommonFields(
         $common->successUrl = $successUrl;
         $common->cancelUrl = $successUrl . '?' . http_build_query(['status' => 'canceled']);
         if (null !== $order) {
-            $common->billing = $this->orderAddressDtoCreator->createBilling($order);
-            $common->shipping = $this->orderAddressDtoCreator->createShipping($order);
+            // Any valid phone number is sent, even from another country than the address. The
+            // creator's strict mode (requireRegionMatch: true) is there for a method that needs it.
+            $common->billing = $this->orderAddressDtoCreator->createBilling($order, requireRegionMatch: false);
+            $common->shipping = $this->orderAddressDtoCreator->createShipping($order, requireRegionMatch: false);
         }
 
         return $common;

src/Upc/PhoneRegionResolver.php

@@ -0,0 +1,48 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PayPlug\SyliusPayPlugPlugin\Upc;
+
+/**
+ * Lists the regions an address's phone number is read in, in order — shared by
+ * OrderAddressDtoCreator (Unified API/Hosted Fields) and the legacy PayPlugPaymentDataCreator.
+ */
+final class PhoneRegionResolver
+{
+    /**
+     * Overseas territories sharing the French numbering, where shoppers often enter a
+     * metropolitan number in national format.
+     */
+    private const FRENCH_NUMBERING_TERRITORIES = ['GP', 'MQ', 'RE', 'GF', 'YT', 'PM', 'BL', 'MF', 'NC', 'PF', 'WF'];
+
+    /**
+     * Territories sharing the +590 numbering with Guadeloupe: libphonenumber attributes a
+     * shared number to the first of these regions it is valid in.
+     */
+    private const SHARED_NUMBERING_REGIONS = [
+        'BL' => ['GP', 'MF'],
+        'MF' => ['GP', 'BL'],
+    ];
+
+    /**
+     * @codeCoverageIgnore
+     */
+    private function __construct()
+    {
+    }
+
+    /**
+     * @return list<string> the address country first, then the regions sharing its numbering, then
+     *      FR for an overseas territory sharing the French numbering
+     */
+    public static function resolve(string $countryCode): array
+    {
+        $countryCode = \strtoupper($countryCode);
+        if (!\in_array($countryCode, self::FRENCH_NUMBERING_TERRITORIES, true)) {
+            return [$countryCode];
+        }
+
+        return [$countryCode, ...(self::SHARED_NUMBERING_REGIONS[$countryCode] ?? []), 'FR'];
+    }
+}

tests/PHPUnit/Creator/PayPlugPaymentDataCreatorTest.php

@@ -127,6 +127,65 @@ public function testFormatNumber_validBelgianNumber_returnsFormattedE164(): void
         self::assertFalse($result['is_mobile']);
     }
 
+    /**
+     * @dataProvider phoneRegionProvider
+     */
+    public function testFormatNumber_readsTheNumberInTheAddressRegionsAndAcceptsForeignNumbers(
+        string $phoneNumber,
+        ?string $isoCode,
+        ?string $expectedPhone,
+        ?bool $expectedIsMobile,
+    ): void {
+        $result = $this->creator->formatNumber($phoneNumber, $isoCode);
+
+        self::assertSame(['phone' => $expectedPhone, 'is_mobile' => $expectedIsMobile], $result);
+    }
+
+    /**
+     * @return iterable<string, array{string, string|null, string|null, bool|null}>
+     */
+    public static function phoneRegionProvider(): iterable
+    {
+        yield 'metropolitan mobile on a MQ address' => ['07 81 23 45 67', 'MQ', '+33781234567', true];
+        yield 'metropolitan mobile on a lowercase mq address' => ['07 81 23 45 67', 'mq', '+33781234567', true];
+        yield 'MQ mobile stays in MQ' => ['0696 12 34 56', 'MQ', '+596696123456', true];
+        yield 'YT landline also valid in FR stays in YT' => ['02 69 61 23 45', 'YT', '+262269612345', false];
+        yield 'shared +590 mobile on a BL address' => ['0690 12 34 56', 'BL', '+590690123456', true];
+        yield 'shared +590 mobile on a MF address' => ['0690123456', 'MF', '+590690123456', true];
+        yield 'shared +590 landline on a MF address' => ['0590871234', 'MF', '+590590871234', false];
+        yield 'FR mobile on a DE address' => ['+33612345678', 'DE', '+33612345678', true];
+        yield 'FR landline on a DE address' => ['+33142345678', 'DE', '+33142345678', false];
+        yield 'FR mobile with 00 prefix on a BE address' => ['0033609860479', 'BE', '+33609860479', true];
+        yield 'BE mobile with 00 prefix on a FR address' => ['0032 470 12 34 56', 'FR', '+32470123456', true];
+        yield 'FR mobile on a FR address' => ['06 12 34 56 78', 'FR', '+33612345678', true];
+        yield 'national number read in DE, not retried as FR' => ['0612345678', 'DE', '+49612345678', false];
+        yield 'national number invalid in BE, not retried as FR' => ['0612345678', 'BE', null, null];
+        yield 'invalid number on a MQ address' => ['0612 34', 'MQ', null, null];
+        yield 'national number without country' => ['0612345678', null, null, null];
+        yield 'international number without country' => ['+33612345678', null, '+33612345678', true];
+    }
+
+    /**
+     * @dataProvider frenchNumberingTerritoryProvider
+     */
+    public function testFormatNumber_withAMetropolitanMobileOnAnOverseasTerritory_returnsItAsFrench(
+        string $isoCode,
+    ): void {
+        $result = $this->creator->formatNumber('0612345678', $isoCode);
+
+        self::assertSame(['phone' => '+33612345678', 'is_mobile' => true], $result);
+    }
+
+    /**
+     * @return iterable<string, array{string}>
+     */
+    public static function frenchNumberingTerritoryProvider(): iterable
+    {
+        foreach (['GP', 'MQ', 'RE', 'GF', 'YT', 'PM', 'BL', 'MF', 'NC', 'PF', 'WF'] as $isoCode) {
+            yield $isoCode => [$isoCode];
+        }
+    }
+
     // -------------------------------------------------------------------------
     // create() — basic fields
     // -------------------------------------------------------------------------
@@ -503,6 +562,18 @@ public function testCreate_withNullPhone_doesNotPopulatePhoneFields(): void
         self::assertNull($details['billing']['landline_phone_number']);
     }
 
+    public function testCreate_withAMetropolitanMobileOnAnOverseasAddress_sendsItAsFrenchInBillingAndShipping(): void
+    {
+        $payment = $this->buildMinimalPayment(1000, 'EUR', 1, 'ORD-1', phone: '07 81 23 45 67', countryCode: 'MQ');
+
+        $details = $this->creator->create($payment);
+
+        self::assertSame('+33781234567', $details['billing']['mobile_phone_number']);
+        self::assertNull($details['billing']['landline_phone_number']);
+        self::assertSame('+33781234567', $details['shipping']['mobile_phone_number']);
+        self::assertNull($details['shipping']['landline_phone_number']);
+    }
+
     // -------------------------------------------------------------------------
     // Helpers
     // -------------------------------------------------------------------------
@@ -516,14 +587,15 @@ private function buildMinimalPayment(
         string $locale = 'fr_FR',
         bool $sameAddress = false,
         ?string $phone = null,
+        string $countryCode = 'FR',
     ): PaymentInterface {
         $customer = $this->createMock(CustomerInterface::class);
         $customer->method('getId')->willReturn($customerId);
         $customer->method('getGender')->willReturn($gender);
         $customer->method('getEmail')->willReturn('test@example.com');
 
-        $billingAddress = $this->buildAddress(1, 'FR', $phone);
-        $shippingAddress = $this->buildAddress($sameAddress ? 1 : 2, 'FR', $phone);
+        $billingAddress = $this->buildAddress(1, $countryCode, $phone);
+        $shippingAddress = $this->buildAddress($sameAddress ? 1 : 2, $countryCode, $phone);
 
         $order = $this->createMock(OrderInterface::class);
         $order->method('getCustomer')->willReturn($customer);