fix: validate newly entered phone numbers

src/BusProNet/Model/Communication.php

@@ -4,6 +4,7 @@ declare(strict_types=1);
 
 namespace App\BusProNet\Model;
 
+use App\Validator\Constraints as AppAssert;
 use Symfony\Component\Validator\Constraints as Assert;
 
 /**
@@ -15,17 +16,47 @@ use Symfony\Component\Validator\Constraints as Assert;
  */
 class Communication
 {
+    #[Assert\When(
+        expression: 'this.isPhoneNewlyEntered()',
+        constraints: [new AppAssert\PhoneNumber()],
+        groups: ['personal_data'],
+    )]
     public ?string $phone = null;
 
     #[Assert\NotBlank(message: 'Bitte angeben', groups: ['personal_data'])]
+    #[Assert\When(
+        expression: 'this.isMobileNewlyEntered()',
+        constraints: [new AppAssert\PhoneNumber()],
+        groups: ['personal_data'],
+    )]
     public ?string $mobile = null;
 
+    // The numbers as loaded from BusPro, which go back unchanged and are exempt from PhoneNumber.
+    public ?string $loadedPhone = null;
+    public ?string $loadedMobile = null;
+
     #[Assert\NotBlank(message: 'Bitte angeben', groups: ['personal_identity'])]
     #[Assert\Email(message: 'Bitte eine gültige Adresse angeben', mode: 'strict', groups: ['personal_identity'])]
     public ?string $email = null;
 
     public bool $newsletter = false;
 
+    public function rememberLoadedNumbers(): void
+    {
+        $this->loadedPhone = $this->phone;
+        $this->loadedMobile = $this->mobile;
+    }
+
+    public function isPhoneNewlyEntered(): bool
+    {
+        return AppAssert\PhoneNumber::isNewlyEntered($this->phone, $this->loadedPhone);
+    }
+
+    public function isMobileNewlyEntered(): bool
+    {
+        return AppAssert\PhoneNumber::isNewlyEntered($this->mobile, $this->loadedMobile);
+    }
+
     /**
      * Converts the communication data to API payload format.
      *

src/BusProNet/XmlParser/PersonalDataParser.php

@@ -50,6 +50,7 @@ class PersonalDataParser extends AbstractParser
             $communication->mobile = $this->getStringOrNullValue($contactDataNode->filterXPath('//telefonmobil'));
             $communication->email = $this->getStringOrNullValue($contactDataNode->filterXPath('//email'));
             $communication->newsletter = $this->getBoolValue($contactDataNode->filterXPath('//newsletter'));
+            $communication->rememberLoadedNumbers();
 
             $personalData->communication = $communication;
         }

src/Form/BookingApplicantType.php

@@ -74,7 +74,6 @@ class BookingApplicantType extends AbstractType
             ->add('mobile', TextType::class, [
                 'label' => 'Telefon (mobil)',
                 'required' => true,
-                'sanitize_html' => true,
                 'attr' => [
                     'autocomplete' => 'leave-me-alone-chrome',
                 ],

src/Form/BookingParticipantType.php

@@ -222,7 +222,6 @@ class BookingParticipantType extends AbstractType
             $form->add('mobile', TextType::class, $this->mergeFieldState([
                 'label' => 'Telefon (mobil)',
                 'required' => false,
-                'sanitize_html' => true,
                 'property_path' => 'participant.mobile',
                 'attr' => [
                     'autocomplete' => 'leave-me-alone-chrome',

src/Form/Model/ParticipantDto.php

@@ -93,8 +93,17 @@ class ParticipantDto
     )]
     public ?string $email = null;
 
+    #[Assert\When(
+        expression: 'this.isMobileNewlyEntered()',
+        constraints: [new AppAssert\PhoneNumber()],
+        groups: ['booking_edit', 'booking_create'],
+    )]
     public ?string $mobile = null;
 
+    // The number as loaded from BusPro, which goes back unchanged and is exempt from PhoneNumber.
+    // Public so it survives the session round trip through __unserialize().
+    public ?string $loadedMobile = null;
+
     #[Assert\Valid(groups: ['booking_edit', 'booking_create'])]
     #[Assert\NotNull(message: 'Bitte Adresse angeben', groups: ['applicant_address'])]
     public ?Address $address = null;
@@ -240,6 +249,7 @@ class ParticipantDto
         $instance->nationality = $personalData->nationality ?: 'D';
         $instance->email = $personalData->communication->email;
         $instance->mobile = $personalData->communication->mobile;
+        $instance->loadedMobile = $personalData->communication->mobile;
         $instance->dateOfBirth = $personalData->dateOfBirth;
         $instance->height = $personalData->height;
         $instance->weight = $personalData->weight;
@@ -254,6 +264,11 @@ class ParticipantDto
         return $instance;
     }
 
+    public function isMobileNewlyEntered(): bool
+    {
+        return AppAssert\PhoneNumber::isNewlyEntered($this->mobile, $this->loadedMobile);
+    }
+
     public function isApplicant(): bool
     {
         return 0 === $this->index;

src/Form/Model/VoucherPurchaseDto.php

@@ -8,6 +8,7 @@ use App\BusProNet\Model\Address;
 use App\BusProNet\Model\PersonalData;
 use App\BusProNet\Model\VoucherPaymentMethod;
 use App\BusProNet\Model\VoucherTemplate;
+use App\Validator\Constraints as AppAssert;
 use Symfony\Component\Validator\Constraints as Assert;
 
 /**
@@ -69,10 +70,24 @@ class VoucherPurchaseDto
     #[Assert\Valid]
     public Address $address;
 
+    #[Assert\When(
+        expression: 'this.isPhoneNewlyEntered()',
+        constraints: [new AppAssert\PhoneNumber()],
+        groups: ['voucher_step_2'],
+    )]
     public ?string $phone = null;
 
+    #[Assert\When(
+        expression: 'this.isMobileNewlyEntered()',
+        constraints: [new AppAssert\PhoneNumber()],
+        groups: ['voucher_step_2'],
+    )]
     public ?string $mobile = null;
 
+    // The numbers prefilled from BusPro, which are exempt from PhoneNumber.
+    public ?string $loadedPhone = null;
+    public ?string $loadedMobile = null;
+
     #[Assert\NotBlank(message: 'Bitte angeben', groups: ['voucher_step_2'])]
     #[Assert\Email(message: 'Bitte eine gültige Adresse angeben', mode: 'strict', groups: ['voucher_step_2'])]
     public ?string $email = null;
@@ -90,6 +105,16 @@ class VoucherPurchaseDto
         $this->address->country = 'D';
     }
 
+    public function isPhoneNewlyEntered(): bool
+    {
+        return AppAssert\PhoneNumber::isNewlyEntered($this->phone, $this->loadedPhone);
+    }
+
+    public function isMobileNewlyEntered(): bool
+    {
+        return AppAssert\PhoneNumber::isNewlyEntered($this->mobile, $this->loadedMobile);
+    }
+
     public function getBuyerFullName(): string
     {
         return trim(sprintf('%s %s', $this->firstName ?? '', $this->lastName ?? ''));
@@ -119,6 +144,8 @@ class VoucherPurchaseDto
         $this->address->country ??= 'D';
         $this->phone = $personalData->communication->phone;
         $this->mobile = $personalData->communication->mobile;
+        $this->loadedPhone = $this->phone;
+        $this->loadedMobile = $this->mobile;
         $this->email = $loginEmail;
     }
 }

src/Form/PersonalDataType.php

@@ -81,12 +81,10 @@ class PersonalDataType extends AbstractType
                 'label' => 'Telefon',
                 'required' => false,
                 'property_path' => 'communication.phone',
-                'sanitize_html' => true,
             ])
             ->add('mobile', TextType::class, [
                 'label' => 'Mobil',
                 'property_path' => 'communication.mobile',
-                'sanitize_html' => true,
             ])
         ;
     }

src/Form/VoucherPurchaseStep2Type.php

@@ -55,12 +55,10 @@ class VoucherPurchaseStep2Type extends AbstractType
             ->add('phone', TextType::class, [
                 'label' => 'Telefon privat',
                 'required' => false,
-                'sanitize_html' => true,
             ])
             ->add('mobile', TextType::class, [
                 'label' => 'Mobil',
                 'required' => false,
-                'sanitize_html' => true,
             ])
         ;
     }

src/Service/ParticipantDataPrefiller.php

@@ -67,6 +67,7 @@ class ParticipantDataPrefiller
         $applicant->dateOfBirth = $prepopulated->dateOfBirth;
         $applicant->email = $prepopulated->email;
         $applicant->mobile = $prepopulated->mobile;
+        $applicant->loadedMobile = $prepopulated->loadedMobile;
         $applicant->address = $prepopulated->address;
         $applicant->height = $prepopulated->height;
         $applicant->weight = $prepopulated->weight;

src/Validator/Constraints/PhoneNumber.php

@@ -0,0 +1,44 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Validator\Constraints;
+
+use Symfony\Component\Validator\Constraints\Regex;
+use Symfony\Component\Validator\Constraints\RegexValidator;
+
+use function Symfony\Component\String\u;
+
+/**
+ * Limits a phone number to digits, spaces, slashes and hyphens, with an optional leading plus.
+ *
+ * Only applied to numbers the user entered or changed: BusPro already holds numbers that break
+ * this rule, and one loaded from BusPro has to go back unchanged, or an edit is rejected for the
+ * mismatch. The DTOs remember the loaded number and gate the constraint with isNewlyEntered().
+ */
+#[\Attribute(\Attribute::TARGET_PROPERTY | \Attribute::TARGET_METHOD | \Attribute::IS_REPEATABLE)]
+class PhoneNumber extends Regex
+{
+    public const PATTERN = '~^\+?[0-9][0-9 /\-]*$~';
+
+    /** @param string[]|null $groups */
+    public function __construct(?array $groups = null, mixed $payload = null)
+    {
+        parent::__construct(
+            pattern: self::PATTERN,
+            message: 'Bitte nur Ziffern, Leerzeichen, /, - und ein führendes + verwenden',
+            groups: $groups,
+            payload: $payload,
+        );
+    }
+
+    public static function isNewlyEntered(?string $value, ?string $loaded): bool
+    {
+        return u($value ?? '')->trim()->toString() !== u($loaded ?? '')->trim()->toString();
+    }
+
+    public function validatedBy(): string
+    {
+        return RegexValidator::class;
+    }
+}

tests/Validator/Constraints/PhoneNumberTest.php

@@ -0,0 +1,108 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Validator\Constraints;
+
+use App\BusProNet\Model\Communication;
+use App\BusProNet\Model\PersonalData;
+use App\Form\Model\ParticipantDto;
+use App\Form\Model\VoucherPurchaseDto;
+use App\Validator\Constraints\PhoneNumber;
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\Validator\Validation;
+use Symfony\Component\Validator\Validator\ValidatorInterface;
+
+class PhoneNumberTest extends TestCase
+{
+    private ValidatorInterface $validator;
+
+    protected function setUp(): void
+    {
+        $this->validator = Validation::createValidatorBuilder()->enableAttributeMapping()->getValidator();
+    }
+
+    /** @return iterable<string, array{string, bool}> */
+    public static function numbers(): iterable
+    {
+        yield 'digits' => ['01711234567', true];
+        yield 'international' => ['+49 171 1234567', true];
+        yield 'hyphen' => ['0171-1234567', true];
+        yield 'slash' => ['0171/1234567', true];
+        yield 'brackets' => ['+49 (0) 171 1234567', false];
+        yield 'sanitized plus' => ['&#43;49 171 1234567', false];
+        yield 'plus inside' => ['0171+1234567', false];
+        yield 'letters' => ['0171 privat', false];
+    }
+
+    #[DataProvider('numbers')]
+    public function testOnlyDigitsSpacesHyphensAndALeadingPlusPass(string $number, bool $valid): void
+    {
+        self::assertSame($valid, 0 === $this->validator->validate($number, new PhoneNumber())->count());
+    }
+
+    public function testAnEnteredParticipantNumberIsValidated(): void
+    {
+        $participant = new ParticipantDto();
+        $participant->mobile = '0171 (1234567)';
+
+        self::assertCount(1, $this->validator->validateProperty($participant, 'mobile', ['booking_create']));
+        self::assertCount(1, $this->validator->validateProperty($participant, 'mobile', ['booking_edit']));
+    }
+
+    /**
+     * A number loaded from BusPro goes back unchanged, so it must not block the form, even when it
+     * is one the rule would reject.
+     */
+    public function testAParticipantNumberLoadedFromBusProIsExemptUntilChanged(): void
+    {
+        $personalData = new PersonalData();
+        $personalData->communication->mobile = '&#43;49 171 1234567';
+        $participant = ParticipantDto::fromPersonalData($personalData);
+
+        self::assertCount(0, $this->validator->validateProperty($participant, 'mobile', ['booking_edit']));
+
+        $participant->mobile = '0171 (1234567)';
+        self::assertCount(1, $this->validator->validateProperty($participant, 'mobile', ['booking_edit']));
+    }
+
+    public function testTheExemptionSurvivesTheSessionRoundTrip(): void
+    {
+        $personalData = new PersonalData();
+        $personalData->communication->mobile = '+49 (171) 1234567';
+        $participant = unserialize(serialize(ParticipantDto::fromPersonalData($personalData)));
+
+        self::assertCount(0, $this->validator->validateProperty($participant, 'mobile', ['booking_edit']));
+    }
+
+    public function testCommunicationExemptsOnlyTheNumbersItWasLoadedWith(): void
+    {
+        $communication = new Communication();
+        $communication->phone = '030 (123456)';
+        $communication->mobile = '+49 (171) 1234567';
+        $communication->rememberLoadedNumbers();
+
+        self::assertCount(0, $this->validator->validate($communication, null, ['personal_data']));
+
+        $communication->phone = '030 (654321)';
+        $violations = $this->validator->validate($communication, null, ['personal_data']);
+        self::assertCount(1, $violations);
+        self::assertSame('phone', $violations->get(0)->getPropertyPath());
+    }
+
+    public function testVoucherBuyerNumbersAreExemptOnlyAsPrefilled(): void
+    {
+        $personalData = new PersonalData();
+        $personalData->communication->phone = '030 (123456)';
+        $personalData->communication->mobile = '+49 (171) 1234567';
+        $dto = new VoucherPurchaseDto();
+        $dto->prefillBuyer($personalData, null);
+
+        self::assertCount(0, $this->validator->validateProperty($dto, 'phone', ['voucher_step_2']));
+        self::assertCount(0, $this->validator->validateProperty($dto, 'mobile', ['voucher_step_2']));
+
+        $dto->mobile = '0171 (1234567)';
+        self::assertCount(1, $this->validator->validateProperty($dto, 'mobile', ['voucher_step_2']));
+    }
+}