From b41b19d4c63cedba758a5c608aca935b7c8f13c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Fromme?= Date: Wed, 3 Jun 2026 17:41:35 +0200 Subject: [PATCH] feat: pre-flight check of agency bookings addresses #869bqxr4q --- .../DataProcessor/BookingDataProcessor.php | 6 +- src/BusProNet/Model/Address.php | 27 ++ .../Booking/Edit/IndexController.php | 29 +- .../Booking/Edit/ParticipantController.php | 84 +++- src/Form/BookingEditType.php | 14 +- src/Form/Model/BookingDto.php | 2 +- src/Form/Model/BookingEditContext.php | 4 +- src/Form/Model/ParticipantDto.php | 72 +++- src/Form/Model/ParticipantEditDto.php | 54 +-- src/Service/BookingEditContextFactory.php | 13 +- src/Service/BookingEditDraftMerger.php | 2 + src/Service/BookingEditPreFlightChecker.php | 150 +++++++ src/Service/ParticipantCardAssembler.php | 160 +++++--- src/Service/ParticipantDataPrefiller.php | 6 - templates/_partials/_alert.html.twig | 6 +- templates/booking/_participant_card.html.twig | 32 +- templates/booking/edit/index.html.twig | 25 +- .../Booking/Edit/IndexControllerTest.php | 382 +++++++++++++++++- .../Edit/ParticipantControllerTest.php | 259 ++++++++++++ tests/Form/BookingEditTypeTest.php | 52 +++ tests/Form/Model/BookingDtoTest.php | 41 ++ tests/Form/Model/ParticipantEditDtoTest.php | 93 +++++ .../Service/BookingEditContextFactoryTest.php | 168 ++------ .../BookingEditPreFlightCheckerTest.php | 341 ++++++++++++++++ .../Service/ParticipantCardAssemblerTest.php | 96 ++++- .../Service/ParticipantDataPrefillerTest.php | 4 +- 26 files changed, 1818 insertions(+), 304 deletions(-) create mode 100644 src/Service/BookingEditPreFlightChecker.php create mode 100644 tests/Controller/Booking/Edit/ParticipantControllerTest.php create mode 100644 tests/Form/BookingEditTypeTest.php create mode 100644 tests/Service/BookingEditPreFlightCheckerTest.php diff --git a/src/BusProNet/DataProcessor/BookingDataProcessor.php b/src/BusProNet/DataProcessor/BookingDataProcessor.php index 2686e34..a669f53 100644 --- a/src/BusProNet/DataProcessor/BookingDataProcessor.php +++ b/src/BusProNet/DataProcessor/BookingDataProcessor.php @@ -15,6 +15,7 @@ use App\Form\Model\ParticipantDto; use App\Form\Model\RoomSelectionDto; use App\Service\BookingPriceCalculator; use App\Service\InsuranceManager; +use function Symfony\Component\String\u; /** * Processes booking form data and converts it into BusProNet API payload format. @@ -78,7 +79,7 @@ class BookingDataProcessor // Copy address if first participant has no street (indicating empty/incomplete address) $isEmpty = null === $participantData->address || null === $participantData->address->street - || '' === trim($participantData->address->street); + || u($participantData->address->street)->trim()->isEmpty(); if ($isEmpty) { $participantData->address = clone $booking->applicant->address; @@ -155,6 +156,9 @@ class BookingDataProcessor // Enrich services with data from travel (especially prices) $this->enrichParticipantServicesFromTravel($participantData, $travel); + // Normalize loaded participant data so the edit flow sees canonical values. + $participantData->normalizeLoadedData(); + $dto->participants[$index] = $participantData; } diff --git a/src/BusProNet/Model/Address.php b/src/BusProNet/Model/Address.php index 85bb5d3..6b4dbca 100644 --- a/src/BusProNet/Model/Address.php +++ b/src/BusProNet/Model/Address.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace App\BusProNet\Model; use Symfony\Component\Validator\Constraints as Assert; +use function Symfony\Component\String\u; /** * Represents a physical address with street, postal code, city, and country information. @@ -29,6 +30,21 @@ class Address #[Assert\NotBlank(message: 'Bitte angeben', groups: ['personal_data'])] public ?string $country = null; + /** + * Normalizes legacy string data loaded from API/session sources. + * + * Trims whitespace and converts empty strings to null so load-time audits + * and form hydration see canonical values. + */ + public function normalize(): void + { + $this->street = $this->normalizeNullableString($this->street); + $this->postCode = $this->normalizeNullableString($this->postCode); + $this->city = $this->normalizeNullableString($this->city); + $this->district = $this->normalizeNullableString($this->district); + $this->country = $this->normalizeNullableString($this->country); + } + /** * Converts the address to API payload format. * @@ -47,4 +63,15 @@ class Address 'land' => $this->country, ]; } + + private function normalizeNullableString(?string $value): ?string + { + if (null === $value) { + return null; + } + + $trimmed = u($value)->trim()->toString(); + + return '' === $trimmed ? null : $trimmed; + } } diff --git a/src/Controller/Booking/Edit/IndexController.php b/src/Controller/Booking/Edit/IndexController.php index b50dc48..43e18a9 100644 --- a/src/Controller/Booking/Edit/IndexController.php +++ b/src/Controller/Booking/Edit/IndexController.php @@ -15,6 +15,7 @@ use App\Service\BookingChangeTracker; use App\Service\BookingEditContextFactory; use App\Service\BookingEditDataLoader; use App\Service\BookingEditDraftManager; +use App\Service\BookingEditPreFlightChecker; use App\Service\BookingEditSubmitter; use App\Service\BookingSessionManager; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; @@ -42,6 +43,7 @@ class IndexController extends AbstractController private readonly BookingSessionManager $bookingSessionService, private readonly BookingChangeTracker $fingerprintService, private readonly BookingEditContextFactory $editContextFactory, + private readonly BookingEditPreFlightChecker $preFlightChecker, private readonly BookingEditSubmitter $formSubmitter, ) { } @@ -107,8 +109,28 @@ class IndexController extends AbstractController $form = $this->createForm(BookingEditType::class, $bookingDto); $form->handleRequest($request); + // Pre-flight check is display-only: it helps users find participants that still need work + // without turning the overview into a blocking validation state. + $missingValueLabelsByParticipantIndex = $this->preFlightChecker + ->findMissingValueLabelsByParticipantIndex($bookingDto); + + // Keep these states separate: + // - $isOverviewSubmitted: the footer form was posted + // - $isOverviewFormValid: Symfony accepted the posted data + // - $hasBlockingValidationErrors: only non-internal bookings block submission + $isOverviewSubmitted = $form->isSubmitted(); + $isOverviewFormValid = false; + + if (true === $isOverviewSubmitted) { + $isOverviewFormValid = $form->isValid(); + } + + $hasBlockingValidationErrors = true === $isOverviewSubmitted + && false === $isOverviewFormValid + && false === $bookingDto->isInternalAgencyBooking(); + // Handle form submission (clicking "Buchung aktualisieren") - if ($form->isSubmitted() && $form->isValid()) { + if (true === $isOverviewSubmitted && (true === $isOverviewFormValid || true === $bookingDto->isInternalAgencyBooking())) { $submissionResult = $this->formSubmitter->handleSubmission($request, $bookingDto, $bookingId, $user); $this->applySubmissionResultFlashes($submissionResult); @@ -119,8 +141,9 @@ class IndexController extends AbstractController $bookingDto, $bookingData, $this->fingerprintService->isDirty($bookingDto), - $form->isSubmitted(), - $form->isSubmitted() && false === $form->isValid(), + $isOverviewSubmitted, + $hasBlockingValidationErrors, + $missingValueLabelsByParticipantIndex, ); $templateData = [ diff --git a/src/Controller/Booking/Edit/ParticipantController.php b/src/Controller/Booking/Edit/ParticipantController.php index aef92b6..31809ea 100644 --- a/src/Controller/Booking/Edit/ParticipantController.php +++ b/src/Controller/Booking/Edit/ParticipantController.php @@ -13,9 +13,11 @@ use App\Htmx\HxTrait; use App\Service\BookingEditContextFactory; use App\Service\BookingEditDataLoader; use App\Service\BookingEditDraftManager; +use App\Service\ParticipantDataPrefiller; use App\Service\BookingSessionManager; use App\Service\ParticipantFormSupport; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; +use Symfony\Component\Form\FormInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Routing\Attribute\Route; @@ -33,6 +35,7 @@ class ParticipantController extends AbstractController private readonly BookingEditDraftManager $draftService, private readonly BookingEditContextFactory $editContextFactory, private readonly BookingSessionManager $bookingSessionService, + private readonly ParticipantDataPrefiller $prepopulationService, private readonly ParticipantFormSupport $participantFormSupportService, ) { } @@ -76,16 +79,26 @@ class ParticipantController extends AbstractController $this->editContextFactory->prepareBookingDto($bookingDto); - $wrapper = $this->participantFormSupportService->createParticipantEditDto($bookingDto, $index); - - $form = $this->createForm( - BookingParticipantType::class, - $wrapper, - $this->participantFormSupportService->getParticipantFormOptions($bookingDto) - ); + $form = $this->createParticipantForm($bookingDto, $index); $form->handleRequest($request); + $isDummyDataFill = $this->prepopulationService->isDummyDataFillRequested( + $bookingDto->participants[$index], + $bookingDto->getMode() + ); + + if (true === $form->isSubmitted() && true === $isDummyDataFill) { + $this->prepopulationService->fillDummyParticipant($bookingDto->participants[$index], $index); + + $this->bookingSessionService->saveBookingDto($request, $bookingDto, BookingDto::MODE_EDIT); + $this->draftService->saveDraft($user, $bookingId, $bookingDto); + + $form = $this->createParticipantForm($bookingDto, $index); + + return $this->renderParticipantForm($form, $index, $bookingDto, $bookingId, $bookingData); + } + $notifications = $this->participantFormSupportService->collectAndClearNotifications($bookingDto); if ($form->isSubmitted() && $form->isValid()) { @@ -96,20 +109,7 @@ class ParticipantController extends AbstractController return $this->redirectToRoute('app_booking_edit', ['bookingId' => $bookingId]); } - $context = $this->editContextFactory->createParticipantContext($bookingDto, $bookingData); - - return $this->render('booking/edit/participant.html.twig', [ - 'form' => $form, - 'participantIndex' => $index, - 'bookingEditContext' => $context, - 'bookingDto' => $context->bookingDto, - 'summaryData' => $context->summaryData, - 'mutableData' => $context->mutableData, - 'refreshRouteName' => 'app_booking_edit_participant_refresh', - 'refreshRouteParams' => ['bookingId' => $bookingId, 'index' => $index], - 'cancelRouteName' => 'app_booking_edit', - 'cancelRouteParams' => ['bookingId' => $bookingId], - ]); + return $this->renderParticipantForm($form, $index, $bookingDto, $bookingId, $bookingData); } /** @@ -190,6 +190,48 @@ class ParticipantController extends AbstractController } } + /** + * @return FormInterface + */ + private function createParticipantForm(BookingDto $bookingDto, int $index): FormInterface + { + $wrapper = $this->participantFormSupportService->createParticipantEditDto($bookingDto, $index); + + return $this->createForm( + BookingParticipantType::class, + $wrapper, + $this->participantFormSupportService->getParticipantFormOptions($bookingDto) + ); + } + + /** + * @param Booking|null $bookingData + * @param FormInterface $form + */ + private function renderParticipantForm( + FormInterface $form, + int $index, + BookingDto $bookingDto, + int $bookingId, + ?Booking $bookingData, + ): Response + { + $context = $this->editContextFactory->createParticipantContext($bookingDto, $bookingData); + + return $this->render('booking/edit/participant.html.twig', [ + 'form' => $form, + 'participantIndex' => $index, + 'bookingEditContext' => $context, + 'bookingDto' => $context->bookingDto, + 'summaryData' => $context->summaryData, + 'mutableData' => $context->mutableData, + 'refreshRouteName' => 'app_booking_edit_participant_refresh', + 'refreshRouteParams' => ['bookingId' => $bookingId, 'index' => $index], + 'cancelRouteName' => 'app_booking_edit', + 'cancelRouteParams' => ['bookingId' => $bookingId], + ]); + } + private function isParticipantCanceled(Booking $bookingData, int $index): bool { return Booking::STATUS_CANCELED === ($bookingData->participantsStatus[$index] ?? null); diff --git a/src/Form/BookingEditType.php b/src/Form/BookingEditType.php index f4db55c..f0b87b0 100644 --- a/src/Form/BookingEditType.php +++ b/src/Form/BookingEditType.php @@ -14,8 +14,9 @@ use Symfony\Component\OptionsResolver\OptionsResolver; * Form type for edit booking validation. * * This form is used in the card-based UI where participants are edited individually. - * This form validates the complete BookingDto before allowing updates, - * ensuring all participants have valid and complete data. + * It only applies blocking validation for non-internal-agency bookings. Internal-agency + * bookings rely on the preflight overview warnings and can still be submitted while + * participants are incomplete. */ /** @extends AbstractType */ class BookingEditType extends AbstractType @@ -34,14 +35,11 @@ class BookingEditType extends AbstractType /** @var BookingDto $data */ $data = $form->getData(); - $groups = ['booking_edit']; - - // Strict validation in edit mode except for internal agency bookings - if (false === $data->isInternalAgencyBooking()) { - $groups[] = 'strict_required'; + if ($data->isInternalAgencyBooking()) { + return ['booking_edit']; } - return $groups; + return ['booking_edit', 'strict_required']; }, ]); } diff --git a/src/Form/Model/BookingDto.php b/src/Form/Model/BookingDto.php index 5ddd6fe..d3ba4cb 100644 --- a/src/Form/Model/BookingDto.php +++ b/src/Form/Model/BookingDto.php @@ -380,7 +380,7 @@ class BookingDto foreach ($this->participants as $participant) { // Temporary cleanup for older serialized participants restored from session. - $participant->normalizeBodyDimensions(); + $participant->normalizeLoadedData(); } // Old sessions (before c373a989) still carry a full Travel object; diff --git a/src/Form/Model/BookingEditContext.php b/src/Form/Model/BookingEditContext.php index fe8a5f9..42994da 100644 --- a/src/Form/Model/BookingEditContext.php +++ b/src/Form/Model/BookingEditContext.php @@ -13,6 +13,7 @@ class BookingEditContext { /** * @param array|null $cardsData + * @param array> $missingValueLabelsByParticipantIndex */ public function __construct( public readonly BookingDto $bookingDto, @@ -20,9 +21,10 @@ class BookingEditContext public readonly ?BookingMutabilityDto $mutableData, public readonly BookingSummaryDto $summaryData, public readonly ?array $cardsData = null, + public readonly array $missingValueLabelsByParticipantIndex = [], public readonly bool $isDirty = false, public readonly bool $isSubmitted = false, - public readonly bool $hasValidationErrors = false, + public readonly bool $hasBlockingValidationErrors = false, ) { } } diff --git a/src/Form/Model/ParticipantDto.php b/src/Form/Model/ParticipantDto.php index e3e1b0f..6535a57 100644 --- a/src/Form/Model/ParticipantDto.php +++ b/src/Form/Model/ParticipantDto.php @@ -12,6 +12,7 @@ use App\BusProNet\Model\Service; use App\Validator\Constraints as AppAssert; use Symfony\Component\Validator\Constraints as Assert; use Symfony\Component\Validator\Context\ExecutionContextInterface; +use function Symfony\Component\String\u; #[AppAssert\Participant(groups: ['booking_edit', 'booking_create'])] class ParticipantDto @@ -374,6 +375,37 @@ class ParticipantDto $this->shoeSize = $this->normalizeBodyDimensionValue($this->shoeSize); } + /** + * Normalizes participant data loaded from API/session sources. + * + * This is deliberately limited to mechanical cleanup: + * - trim whitespace + * - convert empty strings to null + * - normalize legacy body dimension values + * + * It does not fill business-required values. + */ + public function normalizeLoadedData(): void + { + $this->firstName = $this->normalizeNullableString($this->firstName); + $this->lastName = $this->normalizeNullableString($this->lastName); + $this->title = $this->normalizeNullableString($this->title); + $this->gender = $this->normalizeNullableString($this->gender); + $this->nationality = $this->normalizeNullableString($this->nationality); + $this->email = $this->normalizeNullableString($this->email); + $this->mobile = $this->normalizeNullableString($this->mobile); + $this->remarksRoom = $this->normalizeNullableString($this->remarksRoom); + $this->licensePlate = $this->normalizeNullableString($this->licensePlate); + $this->purchaseVoucherCode = $this->normalizeNullableString($this->purchaseVoucherCode); + $this->promoVoucherCode = $this->normalizeNullableString($this->promoVoucherCode); + + if (null !== $this->address) { + $this->address->normalize(); + } + + $this->normalizeBodyDimensions(); + } + /** * Restores the DTO from session/serialization data. * @@ -393,8 +425,7 @@ class ParticipantDto $this->address = new Address(); } - // TODO: Remove after all legacy drafts/session snapshots with body-dimension strings have expired. - $this->normalizeBodyDimensions(); + $this->normalizeLoadedData(); } private function normalizeBodyDimensionValue(mixed $value): ?string @@ -407,7 +438,7 @@ class ParticipantDto return null; } - $value = trim($value); + $value = u($value)->trim()->toString(); if ('' === $value || false === ctype_digit($value)) { return null; } @@ -415,22 +446,37 @@ class ParticipantDto return (string) (int) $value; } + private function normalizeNullableString(?string $value): ?string + { + if (null === $value) { + return null; + } + + $trimmed = u($value)->trim()->toString(); + + return '' === $trimmed ? null : $trimmed; + } + /** - * Validates that the applicant (index 0) has a complete address. + * Validates that the participant has a complete address when required. * - * This callback only applies to the applicant participant. Address validation includes: - * - Address object must exist - * - All required address fields must be filled (street, postCode, city, country) + * In regular create/edit submission flows this only applies to participant 0. + * The edit overview preflight checks address fields separately for the applicant only. */ #[Assert\Callback(groups: ['strict_required'])] public function validateApplicantAddress(ExecutionContextInterface $context): void { - // Only validate applicant's address + // Only validate applicant's address in regular create/edit submission flows. if (0 !== $this->index) { return; } - // Address object is required for applicant + $this->validateAddressFields($context); + } + + private function validateAddressFields(ExecutionContextInterface $context): void + { + // Address object is required for the validated participant if (null === $this->address) { $context->buildViolation('Bitte angeben') ->atPath('address') @@ -440,25 +486,25 @@ class ParticipantDto } // Validate address subfields - if (null === $this->address->street || '' === trim($this->address->street)) { + if (null === $this->address->street || u($this->address->street)->trim()->isEmpty()) { $context->buildViolation('Bitte angeben') ->atPath('address.street') ->addViolation(); } - if (null === $this->address->postCode || '' === trim($this->address->postCode)) { + if (null === $this->address->postCode || u($this->address->postCode)->trim()->isEmpty()) { $context->buildViolation('Bitte angeben') ->atPath('address.postCode') ->addViolation(); } - if (null === $this->address->city || '' === trim($this->address->city)) { + if (null === $this->address->city || u($this->address->city)->trim()->isEmpty()) { $context->buildViolation('Bitte angeben') ->atPath('address.city') ->addViolation(); } - if (null === $this->address->country || '' === trim($this->address->country)) { + if (null === $this->address->country || u($this->address->country)->trim()->isEmpty()) { $context->buildViolation('Bitte angeben') ->atPath('address.country') ->addViolation(); diff --git a/src/Form/Model/ParticipantEditDto.php b/src/Form/Model/ParticipantEditDto.php index 8631b54..a384f19 100644 --- a/src/Form/Model/ParticipantEditDto.php +++ b/src/Form/Model/ParticipantEditDto.php @@ -8,6 +8,7 @@ use App\BusProNet\Constants; use App\Validator\Constraints as AppAssert; use Symfony\Component\Validator\Constraints as Assert; use Symfony\Component\Validator\Context\ExecutionContextInterface; +use function Symfony\Component\String\u; /** * Wrapper DTO for editing a participant within a booking context. @@ -36,14 +37,14 @@ class ParticipantEditDto * (participants aged 0-2 years at travel date). Babies don't qualify for any * ski pass and the field is hidden for them. * - * In edit mode, this validation is skipped as the ski pass is readonly. + * In edit submissions, this validation is skipped as the ski pass is readonly. * * This validation only runs when strict_required group is active. */ #[Assert\Callback(groups: ['strict_required'])] public function validateSkiPassRequired(ExecutionContextInterface $context): void { - // Skip validation in edit mode - ski pass is readonly + // Skip validation in edit submissions - ski pass is readonly there. if (BookingDto::MODE_EDIT === $this->bookingContext->getMode()) { return; } @@ -70,50 +71,27 @@ class ParticipantEditDto * the insurance field is hidden and will be automatically assigned by the bulk * insurance handler. * - * In edit mode, this validation is skipped entirely because insurance data is - * readonly and preserved as-is from the BPN API (the insurance field handler - * does not process insurance in edit mode). + * In edit submissions, this validation is skipped entirely because insurance + * data is readonly and preserved as-is from the BPN API. * * This validation only runs when strict_required group is active. */ #[Assert\Callback(groups: ['strict_required'])] public function validateInsuranceRequired(ExecutionContextInterface $context): void { - // Skip validation in edit mode - insurance is readonly and preserved as-is + // Skip validation in edit submissions - insurance is readonly and preserved as-is. if (BookingDto::MODE_EDIT === $this->bookingContext->getMode()) { return; } - // Insurance is always required for applicant in create mode - if (true === $this->participant->isApplicant()) { - if (null === $this->participant->insurance) { - $context->buildViolation('Bitte auswählen') - ->atPath('participant.insurance') - ->addViolation(); + if (false === $this->participant->isApplicant()) { + $applicant = $this->bookingContext->getParticipant(0); + + if (true === $applicant?->bulkInsuranceBooking) { + return; } - - return; } - // For dependent participants, check if bulk insurance is active - $applicant = $this->bookingContext->getParticipant(0); - if (null === $applicant) { - // Applicant not found - shouldn't happen, but validate insurance to be safe - if (null === $this->participant->insurance) { - $context->buildViolation('Bitte auswählen') - ->atPath('participant.insurance') - ->addViolation(); - } - - return; - } - - // If bulk insurance booking is active, skip validation (field is hidden, will be auto-assigned) - if (true === $applicant->bulkInsuranceBooking) { - return; - } - - // Bulk insurance not active - insurance is required if (null === $this->participant->insurance) { $context->buildViolation('Bitte auswählen') ->atPath('participant.insurance') @@ -138,7 +116,7 @@ class ParticipantEditDto return; } - // Skip validation for applicant (index 0) - applicant's email can be shared with dependents + // In regular create/edit submission flows, the applicant can share an email with dependents. if (true === $this->participant->isApplicant()) { return; } @@ -149,12 +127,12 @@ class ParticipantEditDto } // Skip if email is null or empty (handled by @Email and @NotBlank constraints) - if (null === $this->participant->email || '' === trim($this->participant->email)) { + if (null === $this->participant->email || u($this->participant->email)->trim()->isEmpty()) { return; } // Normalize current participant's email for comparison - $normalizedEmail = strtolower(trim($this->participant->email)); + $normalizedEmail = u($this->participant->email)->trim()->lower()->toString(); // Check against all adult participants in booking context foreach ($this->bookingContext->participants as $index => $otherParticipant) { @@ -169,12 +147,12 @@ class ParticipantEditDto } // Skip null or empty emails - if (null === $otherParticipant->email || '' === trim($otherParticipant->email)) { + if (null === $otherParticipant->email || u($otherParticipant->email)->trim()->isEmpty()) { continue; } // Compare normalized emails - $otherNormalizedEmail = strtolower(trim($otherParticipant->email)); + $otherNormalizedEmail = u($otherParticipant->email)->trim()->lower()->toString(); if ($normalizedEmail === $otherNormalizedEmail) { // Add violation to participant.email path diff --git a/src/Service/BookingEditContextFactory.php b/src/Service/BookingEditContextFactory.php index 1976a9e..f97ee21 100644 --- a/src/Service/BookingEditContextFactory.php +++ b/src/Service/BookingEditContextFactory.php @@ -45,22 +45,29 @@ class BookingEditContextFactory ); } + /** + * @param array> $missingValueLabelsByParticipantIndex + */ public function createOverviewContext( BookingDto $bookingDto, Booking $bookingData, bool $isDirty, bool $isSubmitted, - bool $hasValidationErrors, + bool $hasBlockingValidationErrors, + array $missingValueLabelsByParticipantIndex = [], ): BookingEditContext { + $cardsData = $this->participantCardDataService->getAllCardsDataWithValidation($bookingDto); + return new BookingEditContext( bookingDto: $bookingDto, bookingData: $bookingData, mutableData: BookingMutabilityDto::fromBaseData($this->travelDataService->getMutabilityData($bookingData->dateId)), summaryData: $this->createSummaryData($bookingDto), - cardsData: $this->participantCardDataService->getAllCardsDataWithValidation($bookingDto), + cardsData: $cardsData, + missingValueLabelsByParticipantIndex: $missingValueLabelsByParticipantIndex, isDirty: $isDirty, isSubmitted: $isSubmitted, - hasValidationErrors: $hasValidationErrors, + hasBlockingValidationErrors: $hasBlockingValidationErrors, ); } } diff --git a/src/Service/BookingEditDraftMerger.php b/src/Service/BookingEditDraftMerger.php index cdc2c5f..b0088be 100644 --- a/src/Service/BookingEditDraftMerger.php +++ b/src/Service/BookingEditDraftMerger.php @@ -69,6 +69,8 @@ class BookingEditDraftMerger if (isset($data['services']) && true === is_array($data['services'])) { $this->applyServiceSelections($participant, $data['services'], $travel); } + + $participant->normalizeLoadedData(); } private function canApplyPersonalDataDraft(BookingDto $bookingDto, int $participantIndex, ParticipantDto $participant): bool diff --git a/src/Service/BookingEditPreFlightChecker.php b/src/Service/BookingEditPreFlightChecker.php new file mode 100644 index 0000000..cf58d03 --- /dev/null +++ b/src/Service/BookingEditPreFlightChecker.php @@ -0,0 +1,150 @@ +> + */ + public function findMissingValueLabelsByParticipantIndex(BookingDto $bookingDto): array + { + if (BookingDto::MODE_EDIT !== $bookingDto->getMode()) { + return []; + } + + $missingValueLabelsByParticipantIndex = []; + + foreach ($bookingDto->participants as $index => $participant) { + if (true === $participant->isCanceled()) { + continue; + } + + $missingValueLabels = $this->getMissingValueLabelsForParticipant($bookingDto, $participant); + + if ([] !== $missingValueLabels) { + $missingValueLabelsByParticipantIndex[$index] = $missingValueLabels; + } + } + + return $missingValueLabelsByParticipantIndex; + } + + /** + * @return list + */ + private function getMissingValueLabelsForParticipant(BookingDto $bookingDto, ParticipantDto $participant): array + { + $participantForAudit = new ParticipantEditDto( + participant: $participant, + bookingContext: $bookingDto, + ); + + $violations = $this->validator->validate( + $participantForAudit, + null, + ['booking_edit', 'strict_required'] + ); + + $valueLabels = []; + + foreach ($violations as $violation) { + $valueLabel = $this->mapViolationToValueLabel($violation); + if (null !== $valueLabel) { + $valueLabels[] = $valueLabel; + } + } + + if (false === $bookingDto->isInternalAgencyBooking() && true === $participant->isApplicant()) { + foreach ($this->getMissingValueAddressLabels($participant) as $valueLabel) { + $valueLabels[] = $valueLabel; + } + } + + return array_values(array_unique($valueLabels)); + } + + private function mapViolationToValueLabel(ConstraintViolationInterface $violation): ?string + { + $propertyPath = $violation->getPropertyPath(); + + if (str_starts_with($propertyPath, 'participant.')) { + $propertyPath = substr($propertyPath, strlen('participant.')); + } + + return match ($propertyPath) { + 'firstName' => 'Vorname', + 'lastName' => 'Nachname', + 'gender' => 'Anrede', + 'nationality' => 'Nationalität', + 'dateOfBirth' => 'Geburtsdatum', + 'email' => 'E-Mail', + 'height' => 'Körpergröße', + 'weight' => 'Gewicht', + 'shoeSize' => 'Schuhgröße', + 'assignedRoomId' => 'Zimmer', + 'transportationOutbound' => 'Anreise', + 'pickup' => 'Zustieg', + 'transportationInbound' => 'Abreise', + 'dropOff' => 'Ausstieg', + default => null, + }; + } + + /** + * @return list + */ + private function getMissingValueAddressLabels(ParticipantDto $participant): array + { + if (null === $participant->address) { + return ['Adresse']; + } + + $valueLabels = []; + $violations = $this->validator->validate($participant->address, null, ['personal_data']); + + foreach ($violations as $violation) { + $valueLabel = match ($violation->getPropertyPath()) { + 'street' => 'Straße', + 'postCode' => 'PLZ', + 'city' => 'Ort', + 'country' => 'Land', + default => null, + }; + + if (null !== $valueLabel) { + $valueLabels[] = $valueLabel; + } + } + + return array_values(array_unique($valueLabels)); + } +} diff --git a/src/Service/ParticipantCardAssembler.php b/src/Service/ParticipantCardAssembler.php index edc4a09..b6043c8 100644 --- a/src/Service/ParticipantCardAssembler.php +++ b/src/Service/ParticipantCardAssembler.php @@ -28,6 +28,18 @@ class ParticipantCardAssembler * Get card data for a single participant. */ public function getCardData(BookingDto $bookingDto, int $index): ParticipantCardDataDto + { + return $this->buildCardData($bookingDto, $index); + } + + /** + * @param array|null $precomputedPrices + */ + private function buildCardData( + BookingDto $bookingDto, + int $index, + ?array $precomputedPrices = null, + ): ParticipantCardDataDto { $participant = $bookingDto->participants[$index] ?? null; @@ -45,7 +57,7 @@ class ParticipantCardAssembler $roomName = $this->getRoomName($bookingDto, $participant); // Calculate pricing state for display - $priceData = $this->getPriceData($bookingDto, $index); + $priceData = $this->getPriceData($bookingDto, $index, $precomputedPrices); // Check if participant is canceled $isCanceled = $participant->isCanceled(); @@ -67,9 +79,10 @@ class ParticipantCardAssembler public function getAllCardsData(BookingDto $bookingDto): array { $cardsData = []; + $prices = $this->calculateBulkParticipantPrices($bookingDto); foreach ($bookingDto->participants as $index => $_participant) { - $cardsData[$index] = $this->getCardData($bookingDto, $index); + $cardsData[$index] = $this->buildCardData($bookingDto, $index, $prices); } return $cardsData; @@ -122,8 +135,14 @@ class ParticipantCardAssembler * * Returns a dash marker when the price is zero and no room is assigned, * indicating incomplete configuration rather than a zero-cost booking. + * + * @param array|null $precomputedPrices */ - private function getPriceData(BookingDto $bookingDto, int $index): ParticipantCardPriceDto + private function getPriceData( + BookingDto $bookingDto, + int $index, + ?array $precomputedPrices = null, + ): ParticipantCardPriceDto { // Check if canceled (only possible in edit mode when booking property is set) $isCanceled = ($bookingDto->booking?->participantsStatus[$index] ?? null) === 'S'; @@ -150,7 +169,7 @@ class ParticipantCardAssembler } // For active participants: existing price calculation logic - $prices = $this->priceCalculator->calculateAllParticipantIndividualPrices($bookingDto); + $prices = $precomputedPrices ?? $this->priceCalculator->calculateAllParticipantIndividualPrices($bookingDto); $price = $prices[$index] ?? 0.0; @@ -166,8 +185,92 @@ class ParticipantCardAssembler /** * Get card data for a single participant with validation state. */ - public function getCardDataWithValidation(BookingDto $bookingDto, int $index): ParticipantCardDataDto + public function getCardDataWithValidation( + BookingDto $bookingDto, + int $index, + bool $forceStrictRequired = false, + ): ParticipantCardDataDto { + return $this->getCardDataWithValidationInternal( + $bookingDto, + $index, + $this->determineValidationGroups($bookingDto, $forceStrictRequired) + ); + } + + /** + * Get card data for all participants with validation state. + * + * @return array + */ + public function getAllCardsDataWithValidation(BookingDto $bookingDto, bool $forceStrictRequired = false): array + { + $cardsData = []; + $prices = $this->calculateBulkParticipantPrices($bookingDto); + $validationGroups = $this->determineValidationGroups($bookingDto, $forceStrictRequired); + + foreach ($bookingDto->participants as $index => $_participant) { + $cardsData[$index] = $this->getCardDataWithValidationInternal( + $bookingDto, + $index, + $validationGroups, + $prices + ); + } + + return $cardsData; + } + + /** + * @return array + */ + private function calculateBulkParticipantPrices(BookingDto $bookingDto): array + { + foreach ($bookingDto->participants as $index => $_participant) { + if (($bookingDto->booking?->participantsStatus[$index] ?? null) !== 'S') { + return $this->priceCalculator->calculateAllParticipantIndividualPrices($bookingDto); + } + } + + return []; + } + + /** + * Determines validation groups based on booking mode and applicant mutability. + * + * @return array Validation groups to apply + */ + private function determineValidationGroups(BookingDto $bookingDto, bool $forceStrictRequired = false): array + { + $groups = []; + + // Add mode-specific group + if (BookingDto::MODE_CREATE === $bookingDto->getMode()) { + $groups[] = 'booking_create'; + $groups[] = 'strict_required'; + } else { + $groups[] = 'booking_edit'; + + // Strict validation in edit mode except for internal agency bookings, + // unless the caller explicitly asks for the full validation set. + if (true === $forceStrictRequired || false === $bookingDto->isInternalAgencyBooking()) { + $groups[] = 'strict_required'; + } + } + + return $groups; + } + + /** + * @param array $validationGroups + * @param array|null $precomputedPrices + */ + private function getCardDataWithValidationInternal( + BookingDto $bookingDto, + int $index, + array $validationGroups, + ?array $precomputedPrices = null, + ): ParticipantCardDataDto { $participant = $bookingDto->participants[$index] ?? null; if (null === $participant) { @@ -175,17 +278,13 @@ class ParticipantCardAssembler } // Get basic card data - $cardData = $this->getCardData($bookingDto, $index); + $cardData = $this->buildCardData($bookingDto, $index, $precomputedPrices); - // Wrap participant for validation $wrapper = new ParticipantEditDto( participant: $participant, bookingContext: $bookingDto, ); - // Determine validation groups based on mode and mutability - $validationGroups = $this->determineValidationGroups($bookingDto); - // Validate the wrapper DTO $violations = $this->validator->validate($wrapper, null, $validationGroups); @@ -199,45 +298,4 @@ class ParticipantCardAssembler return $cardData->withValidation($isValid, $errorMessages); } - - /** - * Get card data for all participants with validation state. - * - * @return array - */ - public function getAllCardsDataWithValidation(BookingDto $bookingDto): array - { - $cardsData = []; - - foreach ($bookingDto->participants as $index => $_participant) { - $cardsData[$index] = $this->getCardDataWithValidation($bookingDto, $index); - } - - return $cardsData; - } - - /** - * Determines validation groups based on booking mode and applicant mutability. - * - * @return array Validation groups to apply - */ - private function determineValidationGroups(BookingDto $bookingDto): array - { - $groups = []; - - // Add mode-specific group - if (BookingDto::MODE_CREATE === $bookingDto->getMode()) { - $groups[] = 'booking_create'; - $groups[] = 'strict_required'; - } else { - $groups[] = 'booking_edit'; - - // Strict validation in edit mode except for internal agency bookings - if (false === $bookingDto->isInternalAgencyBooking()) { - $groups[] = 'strict_required'; - } - } - - return $groups; - } } diff --git a/src/Service/ParticipantDataPrefiller.php b/src/Service/ParticipantDataPrefiller.php index 962c23a..a77af38 100644 --- a/src/Service/ParticipantDataPrefiller.php +++ b/src/Service/ParticipantDataPrefiller.php @@ -112,15 +112,9 @@ class ParticipantDataPrefiller /** * Checks if the participant's last name triggers dummy data fill. - * - * Only matches in create mode to avoid accidental triggering during edits. */ public function isDummyDataFillRequested(ParticipantDto $participant, string $mode): bool { - if (BookingDto::MODE_CREATE !== $mode) { - return false; - } - return self::TOKEN === $participant->lastName; } diff --git a/templates/_partials/_alert.html.twig b/templates/_partials/_alert.html.twig index 428a910..56717e1 100644 --- a/templates/_partials/_alert.html.twig +++ b/templates/_partials/_alert.html.twig @@ -3,7 +3,7 @@ {% endif %}
-
+
{% if level == 'error' or level == 'warning' %}
{% if title is defined %} -
+
{{ title }}
{% endif %} {% if messages is defined %} -
+
    {% for message in messages %}
  • diff --git a/templates/booking/_participant_card.html.twig b/templates/booking/_participant_card.html.twig index 0ae987b..5ecfb48 100644 --- a/templates/booking/_participant_card.html.twig +++ b/templates/booking/_participant_card.html.twig @@ -2,10 +2,13 @@ {% set isCanceled = cardData.isCanceled is defined ? cardData.isCanceled : false %} {% set isValid = cardData.isValid is defined ? cardData.isValid : true %} {% set isSubmitted = isSubmitted|default(false) %} +{% set needsAttention = needsAttention|default(false) %} +{% set attentionLabels = attentionLabels|default([]) %} {% set mode = mode|default('create') %} {# Determine display states #} {% set showAsError = not isValid and isSubmitted %} +{% set showAsAttention = needsAttention and not showAsError %} {% set showAsIncomplete = not isValid and not isSubmitted %} {# Build edit URL for linking #} @@ -17,11 +20,13 @@
    -
    = 100, @@ -37,13 +42,26 @@ {% endif %}
    {% if isCanceled %} -
    +
    storniert
    {% elseif showAsError %} -
    +
    unvollständige/fehlerhafte Daten
    + {% elseif showAsAttention %} +
    + bitte prüfen/ergänzen: +
    + {% if attentionLabels|length > 0 %} +
    + {% for label in attentionLabels %} + + {{ label }} + + {% endfor %} +
    + {% endif %} {% else %}
    {{ cardData.roomName }} diff --git a/templates/booking/edit/index.html.twig b/templates/booking/edit/index.html.twig index 60112c3..016e006 100644 --- a/templates/booking/edit/index.html.twig +++ b/templates/booking/edit/index.html.twig @@ -62,18 +62,25 @@ } %} {% endif %} - {# Display validation errors #} - {% if bookingEditContext.hasValidationErrors %} + {# Display blocking issues first; otherwise show the attention scan as a calm warning #} + {% if bookingEditContext.hasBlockingValidationErrors %} {% include '_partials/_alert.html.twig' with { level: 'error', - title: 'Bitte überprüfe die Teilnehmerdaten', - messages: ['Einige Teilnehmer haben noch unvollständige oder fehlerhafte Angaben. Bitte bearbeite die markierten Teilnehmer.'] + title: 'Teilnehmerdaten prüfen', + messages: ['Die Daten einiger Teilnehmer:innen sind noch unvollständig oder fehlerhaft. Bitte bearbeite die markierten Teilnehmer.'] + } %} + {% elseif bookingEditContext.missingValueLabelsByParticipantIndex|length > 0 %} + {% include '_partials/_alert.html.twig' with { + level: 'warning', + title: 'Teilnehmerdaten prüfen', + messages: ['Die Daten von ' ~ (bookingEditContext.missingValueLabelsByParticipantIndex|length) ~ ' Teilnehmer:innen sind noch unvollständig oder fehlerhaft. Bitte bearbeite die markierten Teilnehmer.'] } %} {% endif %} -
    +
    {% for participant in bookingEditContext.bookingDto.participants %} {% set isCanceled = (bookingEditContext.bookingData.participantsStatus[loop.index0] ?? null) == 'S' %} + {% set attentionLabels = bookingEditContext.missingValueLabelsByParticipantIndex[loop.index0]|default([]) %} {% include 'booking/_participant_card.html.twig' with { 'cardData': bookingEditContext.cardsData[loop.index0], 'index': loop.index0, @@ -82,6 +89,8 @@ 'bookingId': bookingEditContext.bookingData.id, 'isCanceled': isCanceled, 'isSubmitted': bookingEditContext.isSubmitted, + 'needsAttention': attentionLabels is not empty, + 'attentionLabels': attentionLabels, } %} {% endfor %}
    @@ -97,13 +106,13 @@ class="inline-flex items-center justify-center bg-gray-200 rounded-md w-8 h-8 md:w-10 md:h-10"> - {% if bookingEditContext.isDirty %} + {% if bookingEditContext.isDirty or bookingEditContext.hasBlockingValidationErrors %} {% endif %} diff --git a/tests/Controller/Booking/Edit/IndexControllerTest.php b/tests/Controller/Booking/Edit/IndexControllerTest.php index 02d3e0a..bd14c04 100644 --- a/tests/Controller/Booking/Edit/IndexControllerTest.php +++ b/tests/Controller/Booking/Edit/IndexControllerTest.php @@ -6,14 +6,20 @@ namespace App\Tests\Controller\Booking\Edit; use App\BusProNet\Model\Booking; use App\BusProNet\Model\Travel; +use App\BusProNet\XmlLoader\AgencyLoader; use App\Controller\Booking\Edit\IndexController; use App\Entity\User; use App\Form\Model\BookingDto; +use App\Form\Model\BookingEditContext; +use App\Form\Model\BookingSummaryDto; +use App\Form\Model\ParticipantCardDataDto; +use App\Form\Model\ParticipantCardPriceDto; use App\Model\BookingEditSubmissionResult; use App\Service\BookingChangeTracker; use App\Service\BookingEditContextFactory; use App\Service\BookingEditDataLoader; use App\Service\BookingEditDraftManager; +use App\Service\BookingEditPreFlightChecker; use App\Service\BookingEditSubmitter; use App\Service\BookingSessionManager; use PHPUnit\Framework\TestCase; @@ -41,6 +47,309 @@ class IndexControllerTest extends TestCase ); } + public function testIndexShowsAttentionWarningWithoutBlockingInternalAgencyOverview(): void + { + $request = Request::create('/bookings/42/edit', 'GET'); + $user = $this->createUser(); + $bookingDto = $this->createBookingDto(); + $bookingDto->agencyCode = AgencyLoader::INTERNAL_AGENCY_CODE; + $bookingDto->participants = [ + $this->createParticipant(), + $this->createParticipant(), + ]; + $bookingData = $this->createBooking(); + + $form = $this->createMock(FormInterface::class); + $form->method('isSubmitted')->willReturn(false); + + $fingerprintService = $this->createMock(BookingChangeTracker::class); + $fingerprintService->expects($this->once()) + ->method('isDirty') + ->with($bookingDto) + ->willReturn(true); + + $dataLoader = $this->createMock(BookingEditDataLoader::class); + $dataLoader->expects($this->once()) + ->method('loadFormData') + ->with($request, 42, $user) + ->willReturn($bookingDto); + $dataLoader->expects($this->once()) + ->method('fetchBookingData') + ->with(42, $user) + ->willReturn($bookingData); + $dataLoader->expects($this->once()) + ->method('isDraftRestored') + ->willReturn(false); + + $preflightChecker = $this->createMock(BookingEditPreFlightChecker::class); + $preflightChecker->expects($this->once()) + ->method('findMissingValueLabelsByParticipantIndex') + ->with($bookingDto) + ->willReturn([1 => ['E-Mail', 'Straße']]); + + $contextFactory = $this->createMock(BookingEditContextFactory::class); + $contextFactory->expects($this->once()) + ->method('createOverviewContext') + ->with($bookingDto, $bookingData, true, false, false, [1 => ['E-Mail', 'Straße']]) + ->willReturn($this->createOverviewContext($bookingDto, $bookingData, true, false, false, [1 => ['E-Mail', 'Straße']])); + + $controller = new TestableIndexController( + $dataLoader, + $this->createMock(BookingEditDraftManager::class), + $this->createMock(BookingSessionManager::class), + $fingerprintService, + $contextFactory, + $preflightChecker, + $this->createMock(BookingEditSubmitter::class), + $user, + $form, + ); + + $response = $controller->index(42, $request); + + $this->assertInstanceOf(Response::class, $response); + $this->assertSame([], $controller->flashes); + $this->assertTrue($controller->renderParameters['bookingEditContext']->isDirty); + $this->assertFalse($controller->renderParameters['bookingEditContext']->hasBlockingValidationErrors); + $this->assertSame( + [1 => ['E-Mail', 'Straße']], + $controller->renderParameters['bookingEditContext']->missingValueLabelsByParticipantIndex + ); + $this->assertTrue($controller->renderParameters['bookingEditContext']->cardsData[0]->isValid); + $this->assertTrue($controller->renderParameters['bookingEditContext']->cardsData[1]->isValid); + } + + public function testIndexShowsAttentionWarningWithoutBlockingNonAgencyOverview(): void + { + $request = Request::create('/bookings/42/edit', 'GET'); + $user = $this->createUser(); + $bookingDto = $this->createBookingDto(); + $bookingDto->agencyCode = 'OTHER'; + $bookingDto->participants = [ + $this->createParticipant(), + $this->createParticipant(), + ]; + $bookingData = $this->createBooking(); + + $form = $this->createMock(FormInterface::class); + $form->method('isSubmitted')->willReturn(false); + + $fingerprintService = $this->createMock(BookingChangeTracker::class); + $fingerprintService->expects($this->once()) + ->method('isDirty') + ->with($bookingDto) + ->willReturn(false); + + $dataLoader = $this->createMock(BookingEditDataLoader::class); + $dataLoader->expects($this->once()) + ->method('loadFormData') + ->with($request, 42, $user) + ->willReturn($bookingDto); + $dataLoader->expects($this->once()) + ->method('fetchBookingData') + ->with(42, $user) + ->willReturn($bookingData); + $dataLoader->expects($this->once()) + ->method('isDraftRestored') + ->willReturn(false); + + $preflightChecker = $this->createMock(BookingEditPreFlightChecker::class); + $preflightChecker->expects($this->once()) + ->method('findMissingValueLabelsByParticipantIndex') + ->with($bookingDto) + ->willReturn([0 => ['E-Mail']]); + + $contextFactory = $this->createMock(BookingEditContextFactory::class); + $contextFactory->expects($this->once()) + ->method('createOverviewContext') + ->with($bookingDto, $bookingData, false, false, false, [0 => ['E-Mail']]) + ->willReturn($this->createOverviewContext($bookingDto, $bookingData, false, false, false, [0 => ['E-Mail']])); + + $controller = new TestableIndexController( + $dataLoader, + $this->createMock(BookingEditDraftManager::class), + $this->createMock(BookingSessionManager::class), + $fingerprintService, + $contextFactory, + $preflightChecker, + $this->createMock(BookingEditSubmitter::class), + $user, + $form, + ); + + $response = $controller->index(42, $request); + + $this->assertInstanceOf(Response::class, $response); + $this->assertSame([], $controller->flashes); + $this->assertFalse($controller->renderParameters['bookingEditContext']->isDirty); + $this->assertFalse($controller->renderParameters['bookingEditContext']->hasBlockingValidationErrors); + $this->assertSame( + [0 => ['E-Mail']], + $controller->renderParameters['bookingEditContext']->missingValueLabelsByParticipantIndex + ); + } + + public function testIndexStillSubmitsInternalAgencyBookingsWhenOverviewIsInvalid(): void + { + $request = Request::create('/bookings/42/edit', 'POST'); + $user = $this->createUser(); + $bookingDto = $this->createBookingDto(); + $bookingDto->agencyCode = AgencyLoader::INTERNAL_AGENCY_CODE; + $bookingDto->participants = [ + $this->createParticipant(), + $this->createParticipant(), + ]; + $bookingData = $this->createBooking(); + + $form = $this->createMock(FormInterface::class); + $form->expects($this->once()) + ->method('handleRequest') + ->with($request) + ->willReturnSelf(); + $form->method('isSubmitted')->willReturn(true); + $form->expects($this->once()) + ->method('isValid') + ->willReturn(false); + + $fingerprintService = $this->createMock(BookingChangeTracker::class); + $fingerprintService->expects($this->never()) + ->method('isDirty'); + + $dataLoader = $this->createMock(BookingEditDataLoader::class); + $dataLoader->expects($this->once()) + ->method('loadFormData') + ->with($request, 42, $user) + ->willReturn($bookingDto); + $dataLoader->expects($this->once()) + ->method('fetchBookingData') + ->with(42, $user) + ->willReturn($bookingData); + $dataLoader->expects($this->once()) + ->method('isDraftRestored') + ->willReturn(false); + + $preflightChecker = $this->createMock(BookingEditPreFlightChecker::class); + $preflightChecker->expects($this->once()) + ->method('findMissingValueLabelsByParticipantIndex') + ->with($bookingDto) + ->willReturn([1 => ['E-Mail', 'Straße']]); + + $submitter = $this->createMock(BookingEditSubmitter::class); + $submitter->expects($this->once()) + ->method('handleSubmission') + ->with($request, $bookingDto, 42, $user) + ->willReturn(new BookingEditSubmissionResult( + BookingEditSubmissionResult::STATUS_SUCCESS, + null, + false + )); + + $contextFactory = $this->createMock(BookingEditContextFactory::class); + $contextFactory->expects($this->never()) + ->method('createOverviewContext'); + + $controller = new TestableIndexController( + $dataLoader, + $this->createMock(BookingEditDraftManager::class), + $this->createMock(BookingSessionManager::class), + $fingerprintService, + $contextFactory, + $preflightChecker, + $submitter, + $user, + $form, + ); + + $response = $controller->index(42, $request); + + $this->assertInstanceOf(RedirectResponse::class, $response); + $this->assertSame('/bookings/42/edit', $response->headers->get('Location')); + $this->assertSame( + [ + ['success', 'Buchung erfolgreich aktualisiert'], + ['booking_edit_notice', 'Bitte ladet euch eine aktualisierte Rechnung herunter, damit ihr stets den aktuellen Stand eurer Buchung vorliegen habt.'], + ], + $controller->flashes + ); + } + + public function testIndexBlocksNonInternalAgencyBookingsWhenOverviewIsInvalid(): void + { + $request = Request::create('/bookings/42/edit', 'POST'); + $user = $this->createUser(); + $bookingDto = $this->createBookingDto(); + $bookingDto->agencyCode = 'OTHER'; + $bookingDto->participants = [ + $this->createParticipant(), + $this->createParticipant(), + ]; + $bookingData = $this->createBooking(); + + $form = $this->createMock(FormInterface::class); + $form->expects($this->once()) + ->method('handleRequest') + ->with($request) + ->willReturnSelf(); + $form->method('isSubmitted')->willReturn(true); + $form->expects($this->once()) + ->method('isValid') + ->willReturn(false); + + $fingerprintService = $this->createMock(BookingChangeTracker::class); + $fingerprintService->expects($this->once()) + ->method('isDirty') + ->with($bookingDto) + ->willReturn(false); + + $dataLoader = $this->createMock(BookingEditDataLoader::class); + $dataLoader->expects($this->once()) + ->method('loadFormData') + ->with($request, 42, $user) + ->willReturn($bookingDto); + $dataLoader->expects($this->once()) + ->method('fetchBookingData') + ->with(42, $user) + ->willReturn($bookingData); + $dataLoader->expects($this->once()) + ->method('isDraftRestored') + ->willReturn(false); + + $preflightChecker = $this->createMock(BookingEditPreFlightChecker::class); + $preflightChecker->expects($this->once()) + ->method('findMissingValueLabelsByParticipantIndex') + ->with($bookingDto) + ->willReturn([0 => ['E-Mail']]); + + $submitter = $this->createMock(BookingEditSubmitter::class); + $submitter->expects($this->never()) + ->method('handleSubmission'); + + $contextFactory = $this->createMock(BookingEditContextFactory::class); + $contextFactory->expects($this->once()) + ->method('createOverviewContext') + ->with($bookingDto, $bookingData, false, true, true, [0 => ['E-Mail']]) + ->willReturn($this->createOverviewContext($bookingDto, $bookingData, false, true, true, [0 => ['E-Mail']])); + + $controller = new TestableIndexController( + $dataLoader, + $this->createMock(BookingEditDraftManager::class), + $this->createMock(BookingSessionManager::class), + $fingerprintService, + $contextFactory, + $preflightChecker, + $submitter, + $user, + $form, + ); + + $response = $controller->index(42, $request); + + $this->assertInstanceOf(Response::class, $response); + $this->assertSame([], $controller->flashes); + $this->assertTrue($controller->renderParameters['bookingEditContext']->hasBlockingValidationErrors); + $this->assertSame([0 => ['E-Mail']], $controller->renderParameters['bookingEditContext']->missingValueLabelsByParticipantIndex); + } + /** * @dataProvider submissionResultProvider * @@ -119,6 +428,49 @@ class IndexControllerTest extends TestCase return $booking; } + private function createParticipant(): \App\Form\Model\ParticipantDto + { + return new \App\Form\Model\ParticipantDto(); + } + + private function createOverviewContext( + BookingDto $bookingDto, + Booking $bookingData, + bool $isDirty, + bool $isSubmitted, + bool $hasBlockingValidationErrors, + array $missingValueLabelsByParticipantIndex = [], + ): BookingEditContext { + return new BookingEditContext( + bookingDto: $bookingDto, + bookingData: $bookingData, + mutableData: null, + summaryData: $this->createMock(BookingSummaryDto::class), + cardsData: [ + 0 => new ParticipantCardDataDto( + 'One', + 'one@example.com', + 'Room A', + new ParticipantCardPriceDto(10.0, false), + false, + true + ), + 1 => new ParticipantCardDataDto( + 'Two', + 'two@example.com', + 'Room B', + new ParticipantCardPriceDto(20.0, false), + false, + true + ), + ], + missingValueLabelsByParticipantIndex: $missingValueLabelsByParticipantIndex, + isDirty: $isDirty, + isSubmitted: $isSubmitted, + hasBlockingValidationErrors: $hasBlockingValidationErrors, + ); + } + /** * @param array $expectedFlashes */ @@ -131,14 +483,15 @@ class IndexControllerTest extends TestCase $bookingDto = $this->createBookingDto(); $bookingData = $this->createBooking(); $form = $this->createMock(FormInterface::class); + $fingerprintService = $this->createMock(BookingChangeTracker::class); + $fingerprintService->expects($this->never()) + ->method('isDirty'); $form->expects($this->once()) ->method('handleRequest') ->with($request) ->willReturnSelf(); - $form->expects($this->once()) - ->method('isSubmitted') - ->willReturn(true); + $form->method('isSubmitted')->willReturn(true); $form->expects($this->once()) ->method('isValid') ->willReturn(true); @@ -153,7 +506,14 @@ class IndexControllerTest extends TestCase ->with(42, $user) ->willReturn($bookingData); $dataLoader->expects($this->once()) - ->method('isDraftRestored'); + ->method('isDraftRestored') + ->willReturn(false); + + $preflightChecker = $this->createMock(BookingEditPreFlightChecker::class); + $preflightChecker->expects($this->once()) + ->method('findMissingValueLabelsByParticipantIndex') + ->with($bookingDto) + ->willReturn([]); $submitter = $this->createMock(BookingEditSubmitter::class); $submitter->expects($this->once()) @@ -165,8 +525,9 @@ class IndexControllerTest extends TestCase $dataLoader, $this->createMock(BookingEditDraftManager::class), $this->createMock(BookingSessionManager::class), - $this->createMock(BookingChangeTracker::class), + $fingerprintService, $this->createMock(BookingEditContextFactory::class), + $preflightChecker, $submitter, $user, $form, @@ -187,6 +548,11 @@ final class TestableIndexController extends IndexController */ public array $flashes = []; + /** + * @var array + */ + public array $renderParameters = []; + /** * @var FormInterface */ @@ -201,6 +567,7 @@ final class TestableIndexController extends IndexController BookingSessionManager $bookingSessionService, BookingChangeTracker $fingerprintService, BookingEditContextFactory $editContextFactory, + BookingEditPreFlightChecker $preFlightChecker, BookingEditSubmitter $formSubmitter, private readonly User $user, FormInterface $form, @@ -213,6 +580,7 @@ final class TestableIndexController extends IndexController $bookingSessionService, $fingerprintService, $editContextFactory, + $preFlightChecker, $formSubmitter, ); } @@ -253,6 +621,8 @@ final class TestableIndexController extends IndexController */ protected function render(string $view, array $parameters = [], ?Response $response = null): Response { - throw new \LogicException('Render should not be called in this test.'); + $this->renderParameters = $parameters; + + return new Response(''); } } diff --git a/tests/Controller/Booking/Edit/ParticipantControllerTest.php b/tests/Controller/Booking/Edit/ParticipantControllerTest.php new file mode 100644 index 0000000..36f65dd --- /dev/null +++ b/tests/Controller/Booking/Edit/ParticipantControllerTest.php @@ -0,0 +1,259 @@ +createUser(); + $bookingDto = $this->createBookingDto(); + $bookingData = $this->createBooking(); + $participant = $bookingDto->participants[0]; + $participant->lastName = ParticipantDataPrefiller::TOKEN; + + $form = $this->createMock(FormInterface::class); + $form->expects($this->once()) + ->method('handleRequest') + ->with($request) + ->willReturnSelf(); + $form->expects($this->once()) + ->method('isSubmitted') + ->willReturn(true); + $form->expects($this->never()) + ->method('isValid'); + + $dataLoader = $this->createMock(BookingEditDataLoader::class); + $dataLoader->expects($this->once()) + ->method('fetchBookingData') + ->with(42, $user) + ->willReturn($bookingData); + + $bookingSessionService = $this->createMock(BookingSessionManager::class); + $bookingSessionService->expects($this->once()) + ->method('getBookingDto') + ->with($request, BookingDto::MODE_EDIT) + ->willReturn($bookingDto); + $bookingSessionService->expects($this->once()) + ->method('saveBookingDto') + ->with($request, $bookingDto, BookingDto::MODE_EDIT); + + $draftService = $this->createMock(BookingEditDraftManager::class); + $draftService->expects($this->once()) + ->method('saveDraft') + ->with($user, 42, $bookingDto); + + $prepopulationService = new ParticipantDataPrefiller( + $this->createMock(ApiClient::class), + $this->createMock(Crypt::class), + $this->createMock(LoggerInterface::class), + ); + + $participantFormSupportService = $this->createMock(ParticipantFormSupport::class); + $participantFormSupportService->expects($this->once()) + ->method('ensureParticipantExists') + ->with($bookingDto, 0) + ->willReturn($participant); + $participantFormSupportService->expects($this->exactly(2)) + ->method('createParticipantEditDto') + ->with($bookingDto, 0) + ->willReturn(new ParticipantEditDto($participant, $bookingDto)); + $participantFormSupportService->expects($this->exactly(2)) + ->method('getParticipantFormOptions') + ->with($bookingDto) + ->willReturn([ + 'booking_context' => $bookingDto, + 'body_dimension_ranges' => [ + 'height_min' => 100, + 'height_max' => 250, + 'weight_min' => 20, + 'weight_max' => 200, + 'shoe_size_min' => 20, + 'shoe_size_max' => 55, + ], + ]); + + $summaryData = $this->createMock(BookingSummaryDto::class); + $context = new BookingEditContext($bookingDto, $bookingData, null, $summaryData); + + $contextFactory = $this->createMock(BookingEditContextFactory::class); + $contextFactory->expects($this->once()) + ->method('prepareBookingDto') + ->with($bookingDto); + $contextFactory->expects($this->once()) + ->method('createParticipantContext') + ->with($bookingDto, $bookingData) + ->willReturn($context); + + $controller = new TestableParticipantController( + $dataLoader, + $draftService, + $contextFactory, + $bookingSessionService, + $prepopulationService, + $participantFormSupportService, + $user, + $form, + ); + + $response = $controller->editParticipant(42, 0, $request); + + $this->assertInstanceOf(Response::class, $response); + $this->assertSame('booking/edit/participant.html.twig', $controller->renderTemplate); + $this->assertSame('Vorname 1', $participant->firstName); + $this->assertSame('Muster 1 14:30', $participant->lastName); + $this->assertSame('teilnehmer.in1@example.com', $participant->email); + $this->assertSame('2006-01-15', $participant->dateOfBirth?->format('Y-m-d')); + $this->assertSame(BookingParticipantType::class, $controller->createdFormTypes[0]); + $this->assertSame(BookingParticipantType::class, $controller->createdFormTypes[1]); + } + + private function createUser(): User + { + return new User('tester@example.com'); + } + + private function createBookingDto(): BookingDto + { + $travel = new Travel(); + $travel->id = 1234; + + $bookingDto = new BookingDto($travel, 77); + $bookingDto->booking = new Booking(); + $bookingDto->booking->id = 42; + $bookingDto->participants = [new ParticipantDto()]; + $bookingDto->participants[0]->index = 0; + + return $bookingDto; + } + + private function createBooking(): Booking + { + $booking = new Booking(); + $booking->id = 42; + $booking->dateId = 1234; + $booking->participantsStatus = ['F']; + + return $booking; + } +} + +final class TestableParticipantController extends ParticipantController +{ + /** + * @var array + */ + public array $createdFormTypes = []; + + public ?string $renderTemplate = null; + + /** + * @var array + */ + public array $renderParameters = []; + + /** + * @var FormInterface + */ + private readonly FormInterface $form; + + /** + * @param FormInterface $form + */ + public function __construct( + BookingEditDataLoader $dataLoader, + BookingEditDraftManager $draftService, + BookingEditContextFactory $editContextFactory, + BookingSessionManager $bookingSessionService, + ParticipantDataPrefiller $prepopulationService, + ParticipantFormSupport $participantFormSupportService, + private readonly User $user, + FormInterface $form, + ) { + $this->form = $form; + + parent::__construct( + $dataLoader, + $draftService, + $editContextFactory, + $bookingSessionService, + $prepopulationService, + $participantFormSupportService, + ); + } + + protected function getUser(): UserInterface + { + return $this->user; + } + + /** + * @return FormInterface + */ + protected function createForm(string $type, mixed $data = null, array $options = []): FormInterface + { + $this->createdFormTypes[] = $type; + + return $this->form; + } + + protected function addFlash(string $type, mixed $message): void + { + } + + /** + * @param array $parameters + */ + protected function redirectToRoute(string $route, array $parameters = [], int $status = 302): RedirectResponse + { + return new RedirectResponse('/'.$route, $status); + } + + /** + * @param array $parameters + */ + protected function render(string $view, array $parameters = [], ?Response $response = null): Response + { + $this->renderTemplate = $view; + $this->renderParameters = $parameters; + + return new Response(''); + } +} diff --git a/tests/Form/BookingEditTypeTest.php b/tests/Form/BookingEditTypeTest.php new file mode 100644 index 0000000..f107f56 --- /dev/null +++ b/tests/Form/BookingEditTypeTest.php @@ -0,0 +1,52 @@ +createForm($this->createBookingDto(true)); + + $validationGroups = $form->getConfig()->getOption('validation_groups'); + + $this->assertSame(['booking_edit'], $validationGroups($form)); + } + + public function testValidationGroupsRemainStrictForNonAgencyBookings(): void + { + $form = $this->createForm($this->createBookingDto(false)); + + $validationGroups = $form->getConfig()->getOption('validation_groups'); + + $this->assertSame(['booking_edit', 'strict_required'], $validationGroups($form)); + } + + private function createForm(BookingDto $bookingDto) + { + return Forms::createFormFactory()->create(BookingEditType::class, $bookingDto); + } + + private function createBookingDto(bool $internalAgency): BookingDto + { + $travel = new Travel(); + $travel->id = 123; + + $bookingDto = new BookingDto($travel, 77); + $bookingDto->booking = new Booking(); + $bookingDto->booking->id = 42; + $bookingDto->agencyCode = $internalAgency ? AgencyLoader::INTERNAL_AGENCY_CODE : 'other'; + + return $bookingDto; + } +} diff --git a/tests/Form/Model/BookingDtoTest.php b/tests/Form/Model/BookingDtoTest.php index 148ad4e..9eeb495 100644 --- a/tests/Form/Model/BookingDtoTest.php +++ b/tests/Form/Model/BookingDtoTest.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace App\Tests\Form\Model; use App\BusProNet\Model\Travel; +use App\BusProNet\Model\Address; use App\Form\Model\BookingDto; use App\Form\Model\ParticipantDto; use PHPUnit\Framework\TestCase; @@ -87,4 +88,44 @@ class BookingDtoTest extends TestCase $this->assertSame('75', $restored->participants[0]->weight); $this->assertSame('43', $restored->participants[0]->shoeSize); } + + public function testUnserializeNormalizesLegacyParticipantStringsOnLoad(): void + { + $travel = new Travel(); + $travel->id = 42; + $travel->dateFrom = new \DateTimeImmutable('2025-06-01'); + $travel->dateTo = new \DateTimeImmutable('2025-06-08'); + + $participant = new ParticipantDto(); + $participant->firstName = ' Max '; + $participant->lastName = ' Mustermann '; + $participant->email = ' max@example.com '; + $participant->remarksRoom = ' '; + $participant->licensePlate = ' B-AB 123 '; + $participant->purchaseVoucherCode = ' ABC-123 '; + $participant->promoVoucherCode = ' PROMO '; + $participant->address = new Address(); + $participant->address->street = ' Test Street 1 '; + $participant->address->postCode = ' 12345 '; + $participant->address->city = ' Test City '; + $participant->address->country = ' DE '; + + $dto = new BookingDto($travel, 1); + $dto->participants = [$participant]; + + $restored = unserialize(serialize($dto)); + + $this->assertInstanceOf(BookingDto::class, $restored); + $this->assertSame('Max', $restored->participants[0]->firstName); + $this->assertSame('Mustermann', $restored->participants[0]->lastName); + $this->assertSame('max@example.com', $restored->participants[0]->email); + $this->assertNull($restored->participants[0]->remarksRoom); + $this->assertSame('B-AB 123', $restored->participants[0]->licensePlate); + $this->assertSame('ABC-123', $restored->participants[0]->purchaseVoucherCode); + $this->assertSame('PROMO', $restored->participants[0]->promoVoucherCode); + $this->assertSame('Test Street 1', $restored->participants[0]->address->street); + $this->assertSame('12345', $restored->participants[0]->address->postCode); + $this->assertSame('Test City', $restored->participants[0]->address->city); + $this->assertSame('DE', $restored->participants[0]->address->country); + } } diff --git a/tests/Form/Model/ParticipantEditDtoTest.php b/tests/Form/Model/ParticipantEditDtoTest.php index 87c636c..169887f 100644 --- a/tests/Form/Model/ParticipantEditDtoTest.php +++ b/tests/Form/Model/ParticipantEditDtoTest.php @@ -597,6 +597,51 @@ class ParticipantEditDtoTest extends TestCase $this->assertCount(0, $skiPassViolations, 'Ski pass validation should be skipped in edit mode'); } + public function testEditSubmissionValidatesApplicantAddressOnlyForFirstParticipant(): void + { + $travel = new Travel(); + $travel->dateFrom = new \DateTimeImmutable('2025-06-01'); + $travel->dateTo = new \DateTimeImmutable('2025-06-08'); + + $bookingDto = new BookingDto($travel, 1); + $bookingDto->booking = new Booking(); + $bookingDto->agencyCode = AgencyLoader::INTERNAL_AGENCY_CODE; + + $participant0 = $this->createAdultParticipant('first@example.com'); + $participant0->index = 0; + $participant0->address = null; + + $participant1 = $this->createAdultParticipant('second@example.com'); + $participant1->index = 1; + $participant1->address = null; + + $bookingDto->participants = [$participant0, $participant1]; + + $wrapper0 = new ParticipantEditDto( + participant: $bookingDto->participants[0], + bookingContext: $bookingDto, + ); + $wrapper1 = new ParticipantEditDto( + participant: $bookingDto->participants[1], + bookingContext: $bookingDto, + ); + + $violations0 = $this->validator->validate($wrapper0, null, ['booking_edit', 'strict_required']); + $addressViolations0 = array_filter( + iterator_to_array($violations0), + fn ($v) => 'address' === $v->getPropertyPath() || 'participant.address' === $v->getPropertyPath() + ); + + $violations1 = $this->validator->validate($wrapper1, null, ['booking_edit', 'strict_required']); + $addressViolations1 = array_filter( + iterator_to_array($violations1), + fn ($v) => 'address' === $v->getPropertyPath() || 'participant.address' === $v->getPropertyPath() + ); + + $this->assertCount(1, $addressViolations0, 'Applicant address should still be required in edit submission'); + $this->assertCount(0, $addressViolations1, 'Non-applicant address should not be required in edit submission'); + } + public function testSkiPassAgeCalculatedAtTravelDate(): void { $travel = new Travel(); @@ -635,6 +680,54 @@ class ParticipantEditDtoTest extends TestCase $this->assertCount(0, $skiPassViolations, 'Age should be calculated at travel date for baby exemption'); } + public function testDependentWithoutInsurancePassesWhenApplicantUsesBulkInsurance(): void + { + $applicant = $this->createAdultParticipant('applicant@example.com'); + $applicant->bulkInsuranceBooking = true; + + $dependent = $this->createAdultParticipant('dependent@example.com'); + $dependent->insurance = null; + + $bookingDto = $this->createBookingDtoWithParticipants([$applicant, $dependent]); + + $wrapper = new ParticipantEditDto( + participant: $bookingDto->participants[1], + bookingContext: $bookingDto, + ); + + $violations = $this->validator->validate($wrapper, null, ['strict_required']); + $insuranceViolations = array_filter( + iterator_to_array($violations), + fn ($v) => 'participant.insurance' === $v->getPropertyPath() + ); + + $this->assertCount(0, $insuranceViolations); + } + + public function testDependentWithoutInsuranceFailsWhenApplicantDoesNotUseBulkInsurance(): void + { + $applicant = $this->createAdultParticipant('applicant@example.com'); + $applicant->bulkInsuranceBooking = false; + + $dependent = $this->createAdultParticipant('dependent@example.com'); + $dependent->insurance = null; + + $bookingDto = $this->createBookingDtoWithParticipants([$applicant, $dependent]); + + $wrapper = new ParticipantEditDto( + participant: $bookingDto->participants[1], + bookingContext: $bookingDto, + ); + + $violations = $this->validator->validate($wrapper, null, ['strict_required']); + $insuranceViolations = array_filter( + iterator_to_array($violations), + fn ($v) => 'participant.insurance' === $v->getPropertyPath() + ); + + $this->assertCount(1, $insuranceViolations); + } + private function createBookingDtoWithParticipants(array $participants): BookingDto { $travel = new Travel(); diff --git a/tests/Service/BookingEditContextFactoryTest.php b/tests/Service/BookingEditContextFactoryTest.php index 8cbc543..a9aa366 100644 --- a/tests/Service/BookingEditContextFactoryTest.php +++ b/tests/Service/BookingEditContextFactoryTest.php @@ -4,14 +4,12 @@ declare(strict_types=1); namespace App\Tests\Service; -use App\BusProNet\Model\BaseData; use App\BusProNet\Model\Booking; -use App\BusProNet\Model\MutableData; use App\BusProNet\Model\Travel; use App\Form\Model\BookingDto; -use App\Form\Model\BookingEditContext; -use App\Form\Model\BookingMutabilityDto; use App\Form\Model\BookingSummaryDto; +use App\Form\Model\ParticipantCardDataDto; +use App\Form\Model\ParticipantCardPriceDto; use App\Service\BookingEditContextFactory; use App\Service\BookingSummaryAssembler; use App\Service\ParticipantCardAssembler; @@ -20,151 +18,61 @@ use PHPUnit\Framework\TestCase; class BookingEditContextFactoryTest extends TestCase { - public function testPrepareBookingDtoRefreshesTravelAvailability(): void + public function testCreateOverviewContextKeepsCardsNeutralAndTracksMissingValueLabels(): void { $travel = new Travel(); - $travel->dateFrom = new \DateTimeImmutable('2030-01-01'); - $travel->dateTo = new \DateTimeImmutable('2030-01-06'); - $bookingDto = new BookingDto($travel, 157047); + $travel->id = 123; - $travelDataService = $this->createMock(TravelDataProvider::class); - $travelDataService->expects($this->once()) - ->method('enrichWithFreshAvailabilities') - ->with($travel); - - $service = new BookingEditContextFactory( - $this->createMock(ParticipantCardAssembler::class), - $this->createMock(BookingSummaryAssembler::class), - $travelDataService, - ); - - $service->prepareBookingDto($bookingDto); - } - - public function testCreateBuildsEditContext(): void - { - $travel = new Travel(); - $travel->dateFrom = new \DateTimeImmutable('2030-01-01'); - $travel->dateTo = new \DateTimeImmutable('2030-01-06'); - - $bookingDto = new BookingDto($travel, 157047); - $bookingDto->booking = new Booking(); + $bookingDto = new BookingDto($travel, 1); + $bookingDto->participants = [ + new \App\Form\Model\ParticipantDto(), + new \App\Form\Model\ParticipantDto(), + ]; $bookingData = new Booking(); - $bookingData->dateId = 1234; + $bookingData->dateId = 123; - $mutableData = new BaseData([ - MutableData::CATEGORY_ADDITIONAL_SERVICES => new MutableData(MutableData::CATEGORY_ADDITIONAL_SERVICES, true, new \DateTimeImmutable('2030-01-02')), - ]); $summaryData = $this->createMock(BookingSummaryDto::class); - $summaryDataService = $this->createMock(BookingSummaryAssembler::class); - $summaryDataService->expects($this->once()) - ->method('getSummaryData') - ->with($bookingDto) - ->willReturn($summaryData); - - $travelDataService = $this->createMock(TravelDataProvider::class); - $travelDataService->expects($this->once()) - ->method('getMutabilityData') - ->with(1234) - ->willReturn($mutableData); - - $service = new BookingEditContextFactory( - $this->createMock(ParticipantCardAssembler::class), - $summaryDataService, - $travelDataService, - ); - - $context = $service->createParticipantContext($bookingDto, $bookingData); - - $this->assertInstanceOf(BookingEditContext::class, $context); - $this->assertSame($bookingDto, $context->bookingDto); - $this->assertSame($bookingData, $context->bookingData); - $this->assertInstanceOf(BookingMutabilityDto::class, $context->mutableData); - $this->assertSame($mutableData->getItemByKey(MutableData::CATEGORY_ADDITIONAL_SERVICES), $context->mutableData->additionalServices); - $this->assertNull($context->mutableData->transportation); - $this->assertSame($summaryData, $context->summaryData); - } - - public function testCreateParticipantContextWithoutBookingDataFallsBackToSummaryOnly(): void - { - $travel = new Travel(); - $travel->dateFrom = new \DateTimeImmutable('2030-01-01'); - $travel->dateTo = new \DateTimeImmutable('2030-01-06'); - - $bookingDto = new BookingDto($travel, 157047); - $summaryData = $this->createMock(BookingSummaryDto::class); - - $summaryDataService = $this->createMock(BookingSummaryAssembler::class); - $summaryDataService->expects($this->once()) - ->method('getSummaryData') - ->with($bookingDto) - ->willReturn($summaryData); - - $travelDataService = $this->createMock(TravelDataProvider::class); - $travelDataService->expects($this->never()) - ->method('getMutabilityData'); - - $service = new BookingEditContextFactory( - $this->createMock(ParticipantCardAssembler::class), - $summaryDataService, - $travelDataService, - ); - - $context = $service->createParticipantContext($bookingDto, null); - - $this->assertInstanceOf(BookingEditContext::class, $context); - $this->assertSame($bookingDto, $context->bookingDto); - $this->assertNull($context->bookingData); - $this->assertNull($context->mutableData); - $this->assertSame($summaryData, $context->summaryData); - } - - public function testCreateOverviewContextBuildsEditOverviewPayload(): void - { - $travel = new Travel(); - $travel->dateFrom = new \DateTimeImmutable('2030-01-01'); - $travel->dateTo = new \DateTimeImmutable('2030-01-06'); - - $bookingDto = new BookingDto($travel, 157047); - $bookingData = new Booking(); - $bookingData->dateId = 1234; - - $mutableData = new BaseData([]); - $summaryData = $this->createMock(BookingSummaryDto::class); - $cardsData = []; - - $participantCardDataService = $this->createMock(ParticipantCardAssembler::class); - $participantCardDataService->expects($this->once()) + $participantCardAssembler = $this->createMock(ParticipantCardAssembler::class); + $participantCardAssembler->expects($this->once()) ->method('getAllCardsDataWithValidation') ->with($bookingDto) - ->willReturn($cardsData); + ->willReturn([ + 0 => new ParticipantCardDataDto('One', 'one@example.com', 'Room A', new ParticipantCardPriceDto(10.0, false), false, true), + 1 => new ParticipantCardDataDto('Two', 'two@example.com', 'Room B', new ParticipantCardPriceDto(20.0, false), false, true), + ]); - $summaryDataService = $this->createMock(BookingSummaryAssembler::class); - $summaryDataService->expects($this->once()) + $summaryAssembler = $this->createMock(BookingSummaryAssembler::class); + $summaryAssembler->expects($this->once()) ->method('getSummaryData') ->with($bookingDto) ->willReturn($summaryData); - $travelDataService = $this->createMock(TravelDataProvider::class); - $travelDataService->expects($this->once()) + $travelDataProvider = $this->createMock(TravelDataProvider::class); + $travelDataProvider->expects($this->once()) ->method('getMutabilityData') - ->with(1234) - ->willReturn($mutableData); + ->with(123) + ->willReturn(null); - $service = new BookingEditContextFactory( - $participantCardDataService, - $summaryDataService, - $travelDataService, + $factory = new BookingEditContextFactory( + $participantCardAssembler, + $summaryAssembler, + $travelDataProvider, ); - $context = $service->createOverviewContext($bookingDto, $bookingData, true, false, true); + $context = $factory->createOverviewContext( + $bookingDto, + $bookingData, + false, + false, + false, + [1 => ['E-Mail', 'Straße']], + ); - $this->assertInstanceOf(BookingEditContext::class, $context); - $this->assertSame($cardsData, $context->cardsData); - $this->assertTrue($context->isDirty); - $this->assertFalse($context->isSubmitted); - $this->assertTrue($context->hasValidationErrors); + $this->assertTrue($context->cardsData[0]->isValid); + $this->assertTrue($context->cardsData[1]->isValid); + $this->assertSame([], $context->cardsData[1]->errorMessages); + $this->assertSame([1 => ['E-Mail', 'Straße']], $context->missingValueLabelsByParticipantIndex); } } diff --git a/tests/Service/BookingEditPreFlightCheckerTest.php b/tests/Service/BookingEditPreFlightCheckerTest.php new file mode 100644 index 0000000..0b89de2 --- /dev/null +++ b/tests/Service/BookingEditPreFlightCheckerTest.php @@ -0,0 +1,341 @@ +dateFrom = CarbonImmutable::now()->addMonth()->toDateTimeImmutable(); + + $bookingDto = new BookingDto($travel, 1); + $bookingDto->booking = new Booking(); + $bookingDto->agencyCode = AgencyLoader::INTERNAL_AGENCY_CODE; + + $bookingDto->participants = [ + $this->createCompleteParticipant(0), + $this->createInternalAgencyParticipantNeedingAttention(1), + ]; + + $checker = new BookingEditPreFlightChecker($this->createValidator()); + + $result = $checker->findMissingValueLabelsByParticipantIndex($bookingDto); + + $this->assertSame( + [1 => ['Vorname', 'Nachname', 'E-Mail']], + $result + ); + } + + public function testScanAttentionDoesNotFlagMissingAddressForNonApplicantParticipantsInNonInternalAgencyEdit(): void + { + $travel = new Travel(); + $travel->dateFrom = CarbonImmutable::now()->addMonth()->toDateTimeImmutable(); + + $bookingDto = new BookingDto($travel, 1); + $bookingDto->booking = new Booking(); + $bookingDto->agencyCode = 'OTHER'; + + $bookingDto->participants = [ + $this->createCompleteParticipant(0), + $this->createParticipantWithMissingAddressOnly(1), + ]; + + $checker = new BookingEditPreFlightChecker($this->createValidator()); + + $result = $checker->findMissingValueLabelsByParticipantIndex($bookingDto); + + $this->assertSame([], $result); + } + + public function testScanAttentionIgnoresBlankBodyDimensionsEvenWhenRentalsAreSelected(): void + { + $travel = new Travel(); + $travel->dateFrom = CarbonImmutable::now()->addMonth()->toDateTimeImmutable(); + + $bookingDto = new BookingDto($travel, 1); + $bookingDto->booking = new Booking(); + $bookingDto->agencyCode = AgencyLoader::INTERNAL_AGENCY_CODE; + + $participant = $this->createCompleteParticipant(0); + $participant->rentals = [$this->createRentalService()]; + $participant->height = null; + $participant->weight = null; + $participant->shoeSize = null; + + $bookingDto->participants = [$participant]; + + $checker = new BookingEditPreFlightChecker($this->createValidator()); + + $result = $checker->findMissingValueLabelsByParticipantIndex($bookingDto); + + $this->assertSame([], $result); + } + + public function testScanAttentionShowsBodyDimensionLabelsWhenProvidedValuesAreOutOfRange(): void + { + $bookingDto = $this->createBookingDto(true); + $participant = $this->createCompleteParticipant(0); + $participant->rentals = [$this->createRentalService()]; + $participant->height = '999'; + $participant->weight = '999'; + $participant->shoeSize = '999'; + + $bookingDto->participants = [$participant]; + + $checker = new BookingEditPreFlightChecker($this->createValidator()); + + $result = $checker->findMissingValueLabelsByParticipantIndex($bookingDto); + + $this->assertSame( + [0 => ['Körpergröße', 'Gewicht', 'Schuhgröße']], + $result + ); + } + + public function testScanAttentionSuppressesOutOfRangeBodyDimensionLabelsWhenSelection1475Applies(): void + { + $bookingDto = $this->createBookingDto(true, [1475]); + $participant = $this->createCompleteParticipant(0); + $participant->rentals = [$this->createRentalService()]; + $participant->height = '999'; + $participant->weight = '999'; + $participant->shoeSize = '999'; + + $bookingDto->participants = [$participant]; + + $checker = new BookingEditPreFlightChecker($this->createValidator()); + + $result = $checker->findMissingValueLabelsByParticipantIndex($bookingDto); + + $this->assertSame([], $result); + } + + public function testScanAttentionShowsPickupAndDropOffLabelsWhenBusTransportRequiresThem(): void + { + $travel = new Travel(); + $travel->dateFrom = CarbonImmutable::now()->addMonth()->toDateTimeImmutable(); + + $bookingDto = new BookingDto($travel, 1); + $bookingDto->booking = new Booking(); + $bookingDto->agencyCode = AgencyLoader::INTERNAL_AGENCY_CODE; + + $participant = $this->createCompleteParticipant(0); + $participant->transportationOutbound = $this->createBusService(); + $participant->transportationInbound = $this->createBusService(); + $participant->differentDropOff = true; + $participant->pickup = null; + $participant->dropOff = null; + + $bookingDto->participants = [$participant]; + + $checker = new BookingEditPreFlightChecker($this->createValidator()); + + $result = $checker->findMissingValueLabelsByParticipantIndex($bookingDto); + + $this->assertSame( + [0 => ['Zustieg', 'Ausstieg']], + $result + ); + } + + private function createCompleteParticipant(int $index): ParticipantDto + { + $participant = new ParticipantDto(); + $participant->index = $index; + $participant->firstName = 'Max'; + $participant->lastName = 'Mustermann'; + $participant->gender = 'M'; + $participant->nationality = 'D'; + $participant->dateOfBirth = new \DateTimeImmutable('1990-01-01'); + $participant->email = 'max@example.com'; + $participant->assignedRoomId = 1; + $participant->transportationOutbound = $this->createTransportService(); + $participant->transportationInbound = $this->createTransportService(); + $participant->address = new Address(); + $participant->address->street = 'Test Street 1'; + $participant->address->postCode = '12345'; + $participant->address->city = 'Test City'; + $participant->address->country = 'DE'; + + return $participant; + } + + private function createInternalAgencyParticipantNeedingAttention(int $index): ParticipantDto + { + $participant = $this->createCompleteParticipant($index); + $participant->firstName = null; + $participant->lastName = null; + $participant->email = null; + $participant->address->street = null; + + return $participant; + } + + private function createParticipantWithMissingAddressOnly(int $index): ParticipantDto + { + $participant = $this->createCompleteParticipant($index); + $participant->email = sprintf('other%d@example.com', $index); + $participant->address = new Address(); + + return $participant; + } + + private function createTransportService(): Service + { + $service = new Service(); + $service->subType = 'TRAIN'; + + return $service; + } + + private function createBusService(): Service + { + $service = new Service(); + $service->subType = 'BUS'; + + return $service; + } + + private function createRentalService(): Service + { + $service = new Service(); + $service->subType = 'VER'; + + return $service; + } + + private function createValidator(): ValidatorInterface + { + $voucherValidator = $this->createMock(VoucherValidator::class); + $bookingPriceCalculator = $this->createMock(BookingPriceCalculator::class); + $participantEligibilityChecker = $this->createMock(ParticipantEligibilityChecker::class); + $participantEligibilityChecker + ->method('isParticipantEligible') + ->willReturn(false); + + $validatorFactory = new class( + $voucherValidator, + $bookingPriceCalculator, + $participantEligibilityChecker, + new ServiceAgeEvaluator(), + ) implements ConstraintValidatorFactoryInterface { + public function __construct( + private readonly VoucherValidator $voucherValidator, + private readonly BookingPriceCalculator $bookingPriceCalculator, + private readonly ParticipantEligibilityChecker $participantEligibilityChecker, + private readonly ServiceAgeEvaluator $serviceAgeEvaluator, + ) { + } + + public function getInstance(Constraint $constraint): ConstraintValidatorInterface + { + $className = $constraint->validatedBy(); + + if (EmailValidator::class === $className) { + return new EmailValidator(Email::VALIDATION_MODE_HTML5); + } + + if (PurchaseVoucherValidator::class === $className) { + return new PurchaseVoucherValidator($this->voucherValidator); + } + + if (PromoVoucherValidator::class === $className) { + return new PromoVoucherValidator($this->voucherValidator, $this->bookingPriceCalculator); + } + + if (MandatoryAdditionalServicesSelectedValidator::class === $className) { + return new MandatoryAdditionalServicesSelectedValidator( + $this->participantEligibilityChecker, + $this->serviceAgeEvaluator + ); + } + + return new $className(); + } + }; + + return Validation::createValidatorBuilder() + ->enableAttributeMapping() + ->setConstraintValidatorFactory($validatorFactory) + ->getValidator(); + } + + /** + * @param list $selectionIds + */ + private function createBookingDto(bool $internalAgency, array $selectionIds = []): BookingDto + { + $travel = new Travel(); + $travel->dateFrom = CarbonImmutable::now()->addMonth()->toDateTimeImmutable(); + $this->applySelectionIds($travel, $selectionIds); + + $bookingDto = new BookingDto($travel, 1); + $bookingDto->booking = new Booking(); + $bookingDto->agencyCode = $internalAgency ? AgencyLoader::INTERNAL_AGENCY_CODE : 'OTHER'; + + return $bookingDto; + } + + /** + * @param list $selectionIds + */ + private function applySelectionIds(Travel $travel, array $selectionIds): void + { + if ([] === $selectionIds) { + return; + } + + $group = new CrmSelectionGroup(); + $group->id = 1; + $group->selections = []; + + foreach ($selectionIds as $selectionId) { + $selection = new CrmSelection(); + $selection->id = $selectionId; + $group->selections[] = $selection; + } + + $travel->selectionGroups = [1 => $group]; + } +} diff --git a/tests/Service/ParticipantCardAssemblerTest.php b/tests/Service/ParticipantCardAssemblerTest.php index 15f6afc..b516126 100644 --- a/tests/Service/ParticipantCardAssemblerTest.php +++ b/tests/Service/ParticipantCardAssemblerTest.php @@ -7,10 +7,14 @@ namespace App\Tests\Service; use Symfony\Component\Validator\ConstraintViolationListInterface; use Symfony\Component\Validator\ConstraintViolationInterface; use Symfony\Component\Validator\ConstraintViolationList; +use App\BusProNet\Model\Booking; use App\BusProNet\Model\Room; +use App\BusProNet\Model\Surcharge; use App\BusProNet\Model\Travel; +use App\BusProNet\XmlLoader\AgencyLoader; use App\Form\Model\BookingDto; use App\Form\Model\ParticipantDto; +use App\Form\Model\ParticipantEditDto; use App\Service\BookingPriceCalculator; use App\Service\ParticipantCardAssembler; use PHPUnit\Framework\TestCase; @@ -278,7 +282,7 @@ class ParticipantCardAssemblerTest extends TestCase // Mock price calculation $this->priceCalculator - ->expects($this->exactly(3)) + ->expects($this->once()) ->method('calculateAllParticipantIndividualPrices') ->with($bookingDto) ->willReturn([450.0, 500.0, 480.0]); @@ -306,6 +310,50 @@ class ParticipantCardAssemblerTest extends TestCase $this->assertFalse($result[2]->price->showDash); } + public function testGetAllCardsDataUsesCanceledSurchargesBeforePrecomputedActivePrices(): void + { + $room = new Room(); + $room->id = 1; + $room->label = 'Doppelzimmer'; + + $travel = new Travel(); + $travel->rooms = [$room]; + + $activeParticipant = new ParticipantDto(); + $activeParticipant->firstName = 'Max'; + $activeParticipant->lastName = 'Mustermann'; + $activeParticipant->assignedRoomId = 1; + + $canceledParticipant = new ParticipantDto(); + $canceledParticipant->firstName = 'Anna'; + $canceledParticipant->lastName = 'Schmidt'; + $canceledParticipant->assignedRoomId = 1; + + $surcharge = new Surcharge(); + $surcharge->mapping = [1]; + $surcharge->individualPrice = [1 => 75.0]; + + $booking = new Booking(); + $booking->participantsStatus = [0 => 'A', 1 => 'S']; + $booking->surcharges = [$surcharge]; + + $bookingDto = new BookingDto($travel, 1); + $bookingDto->booking = $booking; + $bookingDto->participants = [$activeParticipant, $canceledParticipant]; + + $this->priceCalculator + ->expects($this->once()) + ->method('calculateAllParticipantIndividualPrices') + ->with($bookingDto) + ->willReturn([0 => 450.0, 1 => 999.0]); + + $result = $this->service->getAllCardsData($bookingDto); + + $this->assertSame(450.0, $result[0]->price->amount); + $this->assertSame(75.0, $result[1]->price->amount); + $this->assertFalse($result[1]->price->showDash); + } + public function testGetAllCardsDataWithEmptyParticipants(): void { $travel = new Travel(); @@ -471,7 +519,7 @@ class ParticipantCardAssemblerTest extends TestCase $bookingDto->participants = [$participant1, $participant2]; $this->priceCalculator - ->expects($this->exactly(2)) + ->expects($this->once()) ->method('calculateAllParticipantIndividualPrices') ->willReturn([450.0, 500.0]); @@ -494,4 +542,48 @@ class ParticipantCardAssemblerTest extends TestCase $this->assertSame('Max Mustermann', $result[0]->name); $this->assertSame('Anna Schmidt', $result[1]->name); } + + public function testGetCardDataWithValidationCanForceStrictRequiredInEditMode(): void + { + $travel = new Travel(); + $travel->rooms = []; + + $participant = new ParticipantDto(); + $participant->index = 0; + $participant->firstName = null; + $participant->lastName = 'Mustermann'; + $participant->email = 'max@example.com'; + + $bookingDto = new BookingDto($travel, 1); + $bookingDto->agencyCode = AgencyLoader::INTERNAL_AGENCY_CODE; + $bookingDto->booking = new \App\BusProNet\Model\Booking(); + $bookingDto->participants = [$participant]; + + $this->priceCalculator + ->method('calculateAllParticipantIndividualPrices') + ->willReturn([0.0]); + + $violation = $this->createMock(ConstraintViolationInterface::class); + $violation->expects($this->once()) + ->method('getMessage') + ->willReturn('Bitte angeben'); + + $violations = new ConstraintViolationList([$violation]); + + $this->validator + ->expects($this->once()) + ->method('validate') + ->with( + $this->isInstanceOf(\App\Form\Model\ParticipantEditDto::class), + null, + ['booking_edit', 'strict_required'] + ) + ->willReturn($violations); + + $result = $this->service->getCardDataWithValidation($bookingDto, 0, true); + + $this->assertFalse($result->isValid); + $this->assertSame(['Bitte angeben'], $result->errorMessages); + } + } diff --git a/tests/Service/ParticipantDataPrefillerTest.php b/tests/Service/ParticipantDataPrefillerTest.php index 717b601..ae68205 100644 --- a/tests/Service/ParticipantDataPrefillerTest.php +++ b/tests/Service/ParticipantDataPrefillerTest.php @@ -254,13 +254,13 @@ class ParticipantDataPrefillerTest extends TestCase $this->assertFalse($this->service->shouldPrefillApplicant($filled)); } - public function testDummyTokenMatchesOnlyInCreateMode(): void + public function testDummyTokenMatchesInCreateAndEditModes(): void { $participant = new ParticipantDto(); $participant->lastName = ParticipantDataPrefiller::TOKEN; $this->assertTrue($this->service->isDummyDataFillRequested($participant, BookingDto::MODE_CREATE)); - $this->assertFalse($this->service->isDummyDataFillRequested($participant, BookingDto::MODE_EDIT)); + $this->assertTrue($this->service->isDummyDataFillRequested($participant, BookingDto::MODE_EDIT)); } public function testFillDummyParticipantSetsGeneratedData(): void