From 0ee8fafbe16fd578ca2cfbf014271ca4d164ce94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Fromme?= Date: Fri, 16 Oct 2026 17:45:32 +0200 Subject: [PATCH] fix: name missing fields the user cannot edit on personal data page --- src/BusProNet/Model/Communication.php | 4 +- src/BusProNet/Model/PersonalData.php | 8 +- .../Account/PersonalDataController.php | 7 ++ src/Form/RegistrationType.php | 2 +- src/Service/ProfileCompletenessChecker.php | 37 ++++++++- templates/account/personal_data.html.twig | 11 ++- .../Account/PersonalDataControllerTest.php | 48 ++++++++++- .../ProfileCompletenessCheckerTest.php | 82 +++++++++++++++++++ 8 files changed, 187 insertions(+), 12 deletions(-) create mode 100644 tests/Service/ProfileCompletenessCheckerTest.php diff --git a/src/BusProNet/Model/Communication.php b/src/BusProNet/Model/Communication.php index e211476a..a08b1584 100644 --- a/src/BusProNet/Model/Communication.php +++ b/src/BusProNet/Model/Communication.php @@ -20,8 +20,8 @@ class Communication #[Assert\NotBlank(message: 'Bitte angeben', groups: ['personal_data'])] public ?string $mobile = null; - #[Assert\NotBlank(message: 'Bitte angeben', groups: ['personal_data'])] - #[Assert\Email(message: 'Bitte eine gültige Adresse angeben', mode: 'strict', groups: ['personal_data'])] + #[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; diff --git a/src/BusProNet/Model/PersonalData.php b/src/BusProNet/Model/PersonalData.php index f6233b1f..c4562c6e 100644 --- a/src/BusProNet/Model/PersonalData.php +++ b/src/BusProNet/Model/PersonalData.php @@ -23,10 +23,12 @@ class PersonalData public bool $mutable = false; public ?string $status = null; - #[Assert\NotBlank(message: 'Bitte angeben', groups: ['Default', 'personal_data'])] + // personal_identity holds what the user cannot edit in MyE&P (name and the login e-mail), so the + // personal-data form can validate everything else while the completeness check still sees it. + #[Assert\NotBlank(message: 'Bitte angeben', groups: ['Default', 'personal_identity'])] public ?string $name = null; - #[Assert\NotBlank(message: 'Bitte angeben', groups: ['Default', 'personal_data'])] + #[Assert\NotBlank(message: 'Bitte angeben', groups: ['Default', 'personal_identity'])] public ?string $firstName = ''; public ?string $salutation = null; @@ -53,7 +55,7 @@ class PersonalData #[Assert\Valid(groups: ['personal_data'])] public Address $address; - #[Assert\Valid(groups: ['personal_data'])] + #[Assert\Valid(groups: ['personal_data', 'personal_identity'])] public Communication $communication; /** @var list */ diff --git a/src/Controller/Account/PersonalDataController.php b/src/Controller/Account/PersonalDataController.php index 4d12b6c8..52773fc0 100644 --- a/src/Controller/Account/PersonalDataController.php +++ b/src/Controller/Account/PersonalDataController.php @@ -158,9 +158,16 @@ class PersonalDataController extends AbstractController return $this->redirectToRoute('app_personal_data'); } + // The form leaves these out of validation because the user has no field for them, so they + // are reported separately. Skipped for a failed load, which would list every field. + $invalidIdentityFields = null !== $personalData->addressId + ? $this->completenessChecker->invalidIdentityFields($personalData) + : []; + return $this->render('account/personal_data.html.twig', [ 'personalData' => $personalData, 'personalDataForm' => $personalDataForm, + 'invalidIdentityFields' => $invalidIdentityFields, 'newsletterSubscribed' => $newsletterSubscribed, 'newsletterPendingConfirmation' => $newsletterPendingConfirmation, ]); diff --git a/src/Form/RegistrationType.php b/src/Form/RegistrationType.php index 50f6648f..be652492 100644 --- a/src/Form/RegistrationType.php +++ b/src/Form/RegistrationType.php @@ -35,7 +35,7 @@ class RegistrationType extends AbstractType { $resolver->setDefaults([ 'data_class' => RegistrationDto::class, - 'validation_groups' => ['Default', 'personal_data'], + 'validation_groups' => ['Default', 'personal_data', 'personal_identity'], ]); } } diff --git a/src/Service/ProfileCompletenessChecker.php b/src/Service/ProfileCompletenessChecker.php index 5abe652d..7acba1f7 100644 --- a/src/Service/ProfileCompletenessChecker.php +++ b/src/Service/ProfileCompletenessChecker.php @@ -11,13 +11,22 @@ use Symfony\Component\Validator\Validator\ValidatorInterface; * Validates profile completeness for booking requirements. * * Checks whether a user's personal data contains all required fields needed to create - * or edit bookings using Symfony's validation system with the 'personal_data' group. + * or edit bookings using Symfony's validation system with the 'personal_data' and + * 'personal_identity' groups. * Required fields include name, gender, date of birth, nationality, contact information, * and full address data. */ class ProfileCompletenessChecker { - private const VALIDATION_GROUP = 'personal_data'; + private const VALIDATION_GROUPS = ['personal_data', 'personal_identity']; + private const IDENTITY_GROUP = 'personal_identity'; + + // Fields in personal_identity, which the user cannot edit in MyE&P, by violation property path. + private const IDENTITY_FIELD_LABELS = [ + 'firstName' => 'Vorname', + 'name' => 'Nachname', + 'communication.email' => 'E-Mail-Adresse', + ]; public function __construct( private readonly ValidatorInterface $validator, @@ -27,7 +36,7 @@ class ProfileCompletenessChecker /** * Checks if the personal data contains all required fields for booking operations. * - * Validates against the 'personal_data' group which includes: + * Validates against the 'personal_data' and 'personal_identity' groups, which include: * - firstName and name (personal identification) * - gender and dateOfBirth (personal attributes) * - nationality (booking form requirement) @@ -40,8 +49,28 @@ class ProfileCompletenessChecker */ public function isComplete(PersonalData $personalData): bool { - $violations = $this->validator->validate($personalData, null, [self::VALIDATION_GROUP]); + $violations = $this->validator->validate($personalData, null, self::VALIDATION_GROUPS); return 0 === $violations->count(); } + + /** + * Lists the missing or invalid fields the user cannot fix themselves, because only customer + * service may change them in BusPro. + * + * @return list Field labels, in the order of IDENTITY_FIELD_LABELS + */ + public function invalidIdentityFields(PersonalData $personalData): array + { + $paths = []; + foreach ($this->validator->validate($personalData, null, [self::IDENTITY_GROUP]) as $violation) { + $paths[$violation->getPropertyPath()] = true; + } + + return array_values(array_filter( + self::IDENTITY_FIELD_LABELS, + static fn (string $path): bool => isset($paths[$path]), + \ARRAY_FILTER_USE_KEY, + )); + } } diff --git a/templates/account/personal_data.html.twig b/templates/account/personal_data.html.twig index 895a1232..c048efa4 100644 --- a/templates/account/personal_data.html.twig +++ b/templates/account/personal_data.html.twig @@ -7,7 +7,16 @@ Meine
Daten - {% if app.session.get('_profile_completion_redirect') %} + {% if invalidIdentityFields is not empty %} +
+ {% include '_partials/_alert.html.twig' with { + level: 'warning', + messages: [ + 'In deinem Kundenkonto fehlen Angaben oder sind fehlerhaft, die du hier nicht selbst ändern kannst: ' ~ (invalidIdentityFields|join(', ')|e) ~ '. Bitte wende dich an den Kundenservice. Bis dahin ist eine Buchung leider nicht möglich.' + ] + } %} +
+ {% elseif app.session.get('_profile_completion_redirect') %}
{% include '_partials/_alert.html.twig' with { level: 'info', diff --git a/tests/Controller/Account/PersonalDataControllerTest.php b/tests/Controller/Account/PersonalDataControllerTest.php index 1d4b65c6..095fac41 100644 --- a/tests/Controller/Account/PersonalDataControllerTest.php +++ b/tests/Controller/Account/PersonalDataControllerTest.php @@ -56,6 +56,51 @@ class PersonalDataControllerTest extends TestCase self::assertSame(false, $controller->renderedParameters['newsletterPendingConfirmation']); } + public function testIndexListsIdentityFieldsTheUserCannotFix(): void + { + $personalData = $this->createPersonalData('', 'Musterfirma GmbH'); + $completenessChecker = $this->createMock(ProfileCompletenessChecker::class); + $completenessChecker + ->expects(self::once()) + ->method('invalidIdentityFields') + ->with($personalData) + ->willReturn(['Vorname']); + + $controller = $this->createController( + accountClient: $this->createMock(AccountClient::class), + completenessChecker: $completenessChecker, + ); + $controller->accountClient + ->expects(self::once()) + ->method('getPersonalData') + ->willReturn($personalData); + + $controller->index(Request::create('/personal-data', 'GET')); + + self::assertSame(['Vorname'], $controller->renderedParameters['invalidIdentityFields']); + } + + public function testIndexListsNoIdentityFieldsWhenPersonalDataFailedToLoad(): void + { + $completenessChecker = $this->createMock(ProfileCompletenessChecker::class); + $completenessChecker + ->expects(self::never()) + ->method('invalidIdentityFields'); + + $controller = $this->createController( + accountClient: $this->createMock(AccountClient::class), + completenessChecker: $completenessChecker, + ); + $controller->accountClient + ->expects(self::once()) + ->method('getPersonalData') + ->willReturn(new Notification(500, 'no such customer')); + + $controller->index(Request::create('/personal-data', 'GET')); + + self::assertSame([], $controller->renderedParameters['invalidIdentityFields']); + } + public function testIndexStillTracksPendingNewsletterConfirmationWhenSubscribed(): void { $controller = $this->createController( @@ -217,6 +262,7 @@ class PersonalDataControllerTest extends TestCase ?AccountClient $accountClient = null, ?NewsletterManager $newsletterManager = null, ?FormInterface $form = null, + ?ProfileCompletenessChecker $completenessChecker = null, ): TestablePersonalDataController { $user = new User('customer@example.com'); $user->setPassword('secret'); @@ -239,7 +285,7 @@ class PersonalDataControllerTest extends TestCase $accountClient ?? $this->createStub(AccountClient::class), $crypt, $this->createStub(BookingEditDataLoader::class), - $this->createStub(ProfileCompletenessChecker::class), + $completenessChecker ?? $this->createStub(ProfileCompletenessChecker::class), $this->createStub(EntityManagerInterface::class), $newsletterManager ?? $this->createStub(NewsletterManager::class), $this->createStub(LoggerInterface::class), diff --git a/tests/Service/ProfileCompletenessCheckerTest.php b/tests/Service/ProfileCompletenessCheckerTest.php new file mode 100644 index 00000000..aa4d7256 --- /dev/null +++ b/tests/Service/ProfileCompletenessCheckerTest.php @@ -0,0 +1,82 @@ +createValidator()); + $personalData = $this->createCompletePersonalData(); + + self::assertTrue($checker->isComplete($personalData)); + self::assertSame([], $checker->invalidIdentityFields($personalData)); + } + + /** + * A company name in the last name and no first name is what BusPro hands us for some accounts. + */ + public function testMissingFirstNameIsReportedAndKeepsTheProfileIncomplete(): void + { + $checker = new ProfileCompletenessChecker($this->createValidator()); + $personalData = $this->createCompletePersonalData(); + $personalData->firstName = null; + $personalData->name = 'Musterfirma GmbH'; + + self::assertFalse($checker->isComplete($personalData)); + self::assertSame(['Vorname'], $checker->invalidIdentityFields($personalData)); + } + + public function testInvalidIdentityFieldsAreListedInFixedOrder(): void + { + $checker = new ProfileCompletenessChecker($this->createValidator()); + $personalData = $this->createCompletePersonalData(); + $personalData->communication->email = 'not-an-address'; + $personalData->name = ''; + $personalData->firstName = ''; + + self::assertSame(['Vorname', 'Nachname', 'E-Mail-Adresse'], $checker->invalidIdentityFields($personalData)); + } + + public function testFieldsTheUserCanEditAreNotReportedAsIdentityFields(): void + { + $checker = new ProfileCompletenessChecker($this->createValidator()); + $personalData = $this->createCompletePersonalData(); + $personalData->address->street = null; + $personalData->communication->mobile = null; + + self::assertFalse($checker->isComplete($personalData)); + self::assertSame([], $checker->invalidIdentityFields($personalData)); + } + + private function createValidator(): ValidatorInterface + { + return Validation::createValidatorBuilder()->enableAttributeMapping()->getValidator(); + } + + private function createCompletePersonalData(): PersonalData + { + $personalData = new PersonalData(); + $personalData->firstName = 'Mia'; + $personalData->name = 'Muster'; + $personalData->gender = 'W'; + $personalData->nationality = 'D'; + $personalData->dateOfBirth = new \DateTimeImmutable('1990-01-01'); + $personalData->address->street = 'Musterweg 1'; + $personalData->address->postCode = '12345'; + $personalData->address->city = 'Musterstadt'; + $personalData->address->country = 'D'; + $personalData->communication->email = 'mia@example.com'; + $personalData->communication->mobile = '0170 1234567'; + + return $personalData; + } +}