From 63e8a068b358cef0578686dc7aae70b9300d806b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Fromme?= Date: Sun, 5 Apr 2026 18:46:00 +0200 Subject: [PATCH] feat: move prepopulation guards into participant service --- docs/service-simplification-plan.md | 4 +- .../Booking/Create/Step2Controller.php | 19 ++- .../Create/Step2ParticipantController.php | 10 +- src/Form/Service/DummyDataFillService.php | 71 -------- .../BookingParticipantCountService.php | 22 +-- .../ParticipantPrepopulationService.php | 51 ++++++ .../Form/Service/DummyDataFillServiceTest.php | 160 ------------------ .../BookingParticipantCountServiceTest.php | 26 --- .../ParticipantPrepopulationServiceTest.php | 40 +++++ 9 files changed, 114 insertions(+), 289 deletions(-) delete mode 100644 src/Form/Service/DummyDataFillService.php delete mode 100644 tests/Form/Service/DummyDataFillServiceTest.php diff --git a/docs/service-simplification-plan.md b/docs/service-simplification-plan.md index 3114782..89ef305 100644 --- a/docs/service-simplification-plan.md +++ b/docs/service-simplification-plan.md @@ -19,6 +19,8 @@ The codebase is already in a better place than it was at the start of the refact - `BookingService` no longer owns session lifecycle, baseline snapshot handling, return URL management, or room grouping. That work now lives in `BookingSessionService` and `BookingRoomSelectionService`, which keeps the booking orchestration boundary narrower. - `BookingService` still covers hydration, booking bootstrap, service preselection, and booking status rules. +- `BookingParticipantCountService` now handles only participant count shaping. +- `ParticipantPrepopulationService` now owns applicant prefill plus the create-mode dummy-data shortcut. - `BookingPriceCalculatorService` is focused on pricing, but it still sits close to display-oriented behavior in adjacent code paths. - `TravelDataService` remains broad and is likely the next larger boundary after booking orchestration is reduced. @@ -104,7 +106,7 @@ Likely directions, only if justified later: |------|--------|-------| | Participant card DTO cleanup | Done | Card data now uses typed DTOs instead of nested array payloads | | Room label formatting cleanup | Done | Pricing labels now have a dedicated presentation helper | -| Booking service split | In progress | Session lifecycle, baseline snapshot, return URL handling, room grouping, and participant count shaping moved out of `BookingService` | +| Booking service split | In progress | Session lifecycle, baseline snapshot, return URL handling, room grouping, and participant count shaping moved out of `BookingService`; dummy prefill moved into `ParticipantPrepopulationService` | | Pricing service review | Pending | Keep focused on calculation, not rendering | | Travel data service review | Pending | Broad boundary, likely later pass | | Participant field registry review | Deferred | Real orchestration boundary, intentionally left alone for now | diff --git a/src/Controller/Booking/Create/Step2Controller.php b/src/Controller/Booking/Create/Step2Controller.php index 2b1d0c1..4e07dfa 100644 --- a/src/Controller/Booking/Create/Step2Controller.php +++ b/src/Controller/Booking/Create/Step2Controller.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace App\Controller\Booking\Create; +use App\Entity\User; use App\Controller\Booking\Traits\BookingCreateTrait; use App\Controller\Booking\Traits\BookingExceptionHandlerTrait; use App\Form\BookingCreateStep2Type; @@ -65,12 +66,18 @@ class Step2Controller extends AbstractController // Enrich with fresh availability data $this->travelDataService->enrichWithFreshAvailabilities($bookingCreateDto->travel); - // Ensure correct number of participants with prepopulation callback - $this->participantCountService->ensureCorrectNumberOfParticipants( - $bookingCreateDto, - $this->getUser(), - fn ($user, $participant) => $this->prepopulationService->prepopulateApplicantFromUser($user, $participant) - ); + // Ensure correct number of participants first, then prepopulate the applicant if needed. + $this->participantCountService->ensureCorrectNumberOfParticipants($bookingCreateDto); + + $user = $this->getUser(); + if ($user instanceof User + && isset($bookingCreateDto->participants[0]) + && $this->prepopulationService->shouldPrepopulateApplicant($bookingCreateDto->participants[0])) { + $bookingCreateDto->participants[0] = $this->prepopulationService->prepopulateApplicantFromUser( + $user, + $bookingCreateDto->participants[0] + ); + } // Validate room assignments against current selection (handles back-navigation from step 2 to step 1) $this->roomAssignmentService->validateAndResetInvalidAssignments($bookingCreateDto); diff --git a/src/Controller/Booking/Create/Step2ParticipantController.php b/src/Controller/Booking/Create/Step2ParticipantController.php index deb1c11..10c26f5 100644 --- a/src/Controller/Booking/Create/Step2ParticipantController.php +++ b/src/Controller/Booking/Create/Step2ParticipantController.php @@ -6,12 +6,12 @@ namespace App\Controller\Booking\Create; use App\Form\BookingParticipantType; use App\Form\Model\BookingDto; -use App\Form\Service\DummyDataFillService; use App\Htmx\HxTrait; use App\Service\BookingService; use App\Service\BookingSessionService; use App\Service\BookingSummaryDataService; use App\Service\ParticipantFormSupportService; +use App\Service\ParticipantPrepopulationService; use App\Service\TravelDataService; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Component\Form\FormInterface; @@ -31,7 +31,7 @@ class Step2ParticipantController extends AbstractController private readonly BookingSessionService $bookingSessionService, private readonly BookingSummaryDataService $summaryDataService, private readonly TravelDataService $travelDataService, - private readonly DummyDataFillService $dummyDataFillService, + private readonly ParticipantPrepopulationService $prepopulationService, private readonly ParticipantFormSupportService $participantFormSupportService, ) { } @@ -62,11 +62,11 @@ class Step2ParticipantController extends AbstractController $isSubmitted = $form->isSubmitted(); $isDummyDataFill = $this - ->dummyDataFillService - ->isTokenMatch($bookingDto->participants[$index], $bookingDto->getMode()) + ->prepopulationService + ->isDummyDataFillRequested($bookingDto->participants[$index], $bookingDto->getMode()) ; if (true === $isSubmitted && true === $isDummyDataFill) { - $this->dummyDataFillService->fill($bookingDto->participants[$index], $index); + $this->prepopulationService->fillDummyParticipant($bookingDto->participants[$index], $index); $this->bookingService->preselectDefaultServices($bookingDto); $this->bookingService->applyCreateBookingStatusRules($bookingDto); diff --git a/src/Form/Service/DummyDataFillService.php b/src/Form/Service/DummyDataFillService.php deleted file mode 100644 index 7d84534..0000000 --- a/src/Form/Service/DummyDataFillService.php +++ /dev/null @@ -1,71 +0,0 @@ -lastName; - } - - /** - * Fills the participant DTO with generated dummy personal data. - * - * Generates firstName, lastName, email, mobile, date of birth and address - * values based on the participant number (1-based). The lastName includes - * the current time for easy identification of test bookings. - * - * @param ParticipantDto $participant The participant DTO to fill - * @param int $participantIndex The zero-based participant index - */ - public function fill(ParticipantDto $participant, int $participantIndex): void - { - $participantNumber = $participantIndex + 1; - $now = CarbonImmutable::now(); - - $participant->firstName = sprintf('Vorname %d', $participantNumber); - $participant->lastName = sprintf('Muster %d %s', $participantNumber, $now->format('H:i')); - $participant->mobile = '0171/111111'; - $participant->email = sprintf('teilnehmer.in%d@example.com', $participantNumber); - $participant->dateOfBirth = $now->subYears(20)->toImmutable(); - - $address = new Address(); - $address->street = 'Musterstr. 123'; - $address->postCode = '99999'; - $address->city = 'MusterOrt'; - $address->country = 'Deutschland'; - $participant->address = $address; - } -} diff --git a/src/Service/BookingParticipantCountService.php b/src/Service/BookingParticipantCountService.php index 30ad222..b5ba0e8 100644 --- a/src/Service/BookingParticipantCountService.php +++ b/src/Service/BookingParticipantCountService.php @@ -7,7 +7,6 @@ namespace App\Service; use App\BusProNet\Model\Travel; use App\Form\Model\BookingDto; use App\Form\Model\ParticipantDto; -use Symfony\Component\Security\Core\User\UserInterface; /** * Keeps participant-count shaping separate from booking orchestration. @@ -17,13 +16,9 @@ class BookingParticipantCountService /** * Ensures the booking DTO has the expected number of participant objects. * - * @param callable|null $prepopulateCallback fn(UserInterface, ParticipantDto): ParticipantDto */ - public function ensureCorrectNumberOfParticipants( - BookingDto $bookingDto, - ?UserInterface $user = null, - ?callable $prepopulateCallback = null, - ): void { + public function ensureCorrectNumberOfParticipants(BookingDto $bookingDto): void + { $participantsCount = $this->calculateParticipantsCount($bookingDto->roomSelections, $bookingDto->travel); $existingParticipants = $bookingDto->participants; @@ -33,11 +28,6 @@ class BookingParticipantCountService $participant = $existingParticipants[$i] ?? new ParticipantDto(); $participant->index = $i; - // Prepopulate applicant from authenticated user (index 0 only) - if (0 === $i && null !== $user && null !== $prepopulateCallback && $this->shouldPrepopulate($participant)) { - $participant = $prepopulateCallback($user, $participant); - } - $bookingDto->participants[$i] = $participant; } } @@ -63,12 +53,4 @@ class BookingParticipantCountService return $participantsCount; } - - /** - * Only prepopulate if the participant is fresh. - */ - private function shouldPrepopulate(ParticipantDto $participant): bool - { - return null === $participant->firstName || '' === $participant->firstName; - } } diff --git a/src/Service/ParticipantPrepopulationService.php b/src/Service/ParticipantPrepopulationService.php index f6398e2..ede6c9d 100644 --- a/src/Service/ParticipantPrepopulationService.php +++ b/src/Service/ParticipantPrepopulationService.php @@ -5,10 +5,13 @@ declare(strict_types=1); namespace App\Service; use App\BusProNet\ApiClient; +use App\BusProNet\Model\Address; use App\BusProNet\Model\Notification; use App\Entity\User; +use App\Form\Model\BookingDto; use App\Form\Model\ParticipantDto; use App\Security\Crypt; +use Carbon\CarbonImmutable; use Psr\Log\LoggerInterface; /** @@ -20,6 +23,8 @@ use Psr\Log\LoggerInterface; */ class ParticipantPrepopulationService { + public const TOKEN = '#KUN#'; + public function __construct( private readonly ApiClient $apiClient, private readonly Crypt $crypt, @@ -94,4 +99,50 @@ class ParticipantPrepopulationService return $applicant; } } + + /** + * Determines whether the applicant should be prepopulated. + * + * Only prepopulates if the participant is fresh. + */ + public function shouldPrepopulateApplicant(ParticipantDto $applicant): bool + { + return null === $applicant->firstName || '' === $applicant->firstName; + } + + /** + * 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; + } + + /** + * Fills the participant DTO with generated dummy personal data. + */ + public function fillDummyParticipant(ParticipantDto $participant, int $participantIndex): void + { + $participantNumber = $participantIndex + 1; + $now = CarbonImmutable::now(); + + $participant->firstName = sprintf('Vorname %d', $participantNumber); + $participant->lastName = sprintf('Muster %d %s', $participantNumber, $now->format('H:i')); + $participant->mobile = '0171/111111'; + $participant->email = sprintf('teilnehmer.in%d@example.com', $participantNumber); + $participant->dateOfBirth = $now->subYears(20)->toImmutable(); + + $address = new Address(); + $address->street = 'Musterstr. 123'; + $address->postCode = '99999'; + $address->city = 'MusterOrt'; + $address->country = 'Deutschland'; + $participant->address = $address; + } } diff --git a/tests/Form/Service/DummyDataFillServiceTest.php b/tests/Form/Service/DummyDataFillServiceTest.php deleted file mode 100644 index b78796c..0000000 --- a/tests/Form/Service/DummyDataFillServiceTest.php +++ /dev/null @@ -1,160 +0,0 @@ -service = new DummyDataFillService(); - } - - public function testTokenMatchInCreateMode(): void - { - $participant = new ParticipantDto(); - $participant->lastName = DummyDataFillService::TOKEN; - - $this->assertTrue($this->service->isTokenMatch($participant, BookingDto::MODE_CREATE)); - } - - public function testTokenMatchReturnsFalseInEditMode(): void - { - $participant = new ParticipantDto(); - $participant->lastName = DummyDataFillService::TOKEN; - - $this->assertFalse($this->service->isTokenMatch($participant, BookingDto::MODE_EDIT)); - } - - public function testNonMatchingLastNameReturnsFalse(): void - { - $participant = new ParticipantDto(); - $participant->lastName = 'Schmidt'; - - $this->assertFalse($this->service->isTokenMatch($participant, BookingDto::MODE_CREATE)); - } - - public function testNullLastNameReturnsFalse(): void - { - $participant = new ParticipantDto(); - $participant->lastName = null; - - $this->assertFalse($this->service->isTokenMatch($participant, BookingDto::MODE_CREATE)); - } - - public function testFillSetsFirstNameWithParticipantNumber(): void - { - $participant = new ParticipantDto(); - - $this->service->fill($participant, 0); - - $this->assertSame('Vorname 1', $participant->firstName); - } - - public function testFillSetsLastNameWithParticipantNumberAndTime(): void - { - CarbonImmutable::setTestNow(CarbonImmutable::create(2026, 1, 15, 14, 30)); - - $participant = new ParticipantDto(); - - $this->service->fill($participant, 0); - - $this->assertSame('Muster 1 14:30', $participant->lastName); - - CarbonImmutable::setTestNow(); - } - - public function testFillSetsMobilePhone(): void - { - $participant = new ParticipantDto(); - - $this->service->fill($participant, 0); - - $this->assertSame('0171/111111', $participant->mobile); - } - - public function testFillSetsEmailWithParticipantNumber(): void - { - $participant = new ParticipantDto(); - - $this->service->fill($participant, 0); - - $this->assertSame('teilnehmer.in1@example.com', $participant->email); - } - - public function testFillUsesOneBasedParticipantNumber(): void - { - CarbonImmutable::setTestNow(CarbonImmutable::create(2026, 1, 15, 9, 5)); - - $participant0 = new ParticipantDto(); - $participant1 = new ParticipantDto(); - $participant4 = new ParticipantDto(); - - $this->service->fill($participant0, 0); - $this->service->fill($participant1, 1); - $this->service->fill($participant4, 4); - - $this->assertSame('Vorname 1', $participant0->firstName); - $this->assertSame('Muster 1 09:05', $participant0->lastName); - $this->assertSame('teilnehmer.in1@example.com', $participant0->email); - - $this->assertSame('Vorname 2', $participant1->firstName); - $this->assertSame('Muster 2 09:05', $participant1->lastName); - $this->assertSame('teilnehmer.in2@example.com', $participant1->email); - - $this->assertSame('Vorname 5', $participant4->firstName); - $this->assertSame('Muster 5 09:05', $participant4->lastName); - $this->assertSame('teilnehmer.in5@example.com', $participant4->email); - - CarbonImmutable::setTestNow(); - } - - public function testFillSetsDateOfBirthToTwentyYearsAgo(): void - { - CarbonImmutable::setTestNow(CarbonImmutable::create(2026, 3, 10, 12, 0)); - - $participant = new ParticipantDto(); - - $this->service->fill($participant, 0); - - $this->assertInstanceOf(\DateTimeImmutable::class, $participant->dateOfBirth); - $this->assertSame('2006-03-10', $participant->dateOfBirth->format('Y-m-d')); - - CarbonImmutable::setTestNow(); - } - - public function testFillSetsAddress(): void - { - $participant = new ParticipantDto(); - - $this->service->fill($participant, 0); - - $this->assertNotNull($participant->address); - $this->assertSame('Musterstr. 123', $participant->address->street); - $this->assertSame('99999', $participant->address->postCode); - $this->assertSame('MusterOrt', $participant->address->city); - $this->assertSame('Deutschland', $participant->address->country); - } - - public function testFillSetsAddressForAllParticipants(): void - { - $participant0 = new ParticipantDto(); - $participant3 = new ParticipantDto(); - - $this->service->fill($participant0, 0); - $this->service->fill($participant3, 3); - - $this->assertNotNull($participant0->address); - $this->assertNotNull($participant3->address); - $this->assertSame('Musterstr. 123', $participant3->address->street); - } -} diff --git a/tests/Service/BookingParticipantCountServiceTest.php b/tests/Service/BookingParticipantCountServiceTest.php index c7c693f..5d78995 100644 --- a/tests/Service/BookingParticipantCountServiceTest.php +++ b/tests/Service/BookingParticipantCountServiceTest.php @@ -12,7 +12,6 @@ use App\Form\Model\ParticipantDto; use App\Form\Model\RoomSelectionDto; use App\Service\BookingParticipantCountService; use PHPUnit\Framework\TestCase; -use Symfony\Component\Security\Core\User\UserInterface; class BookingParticipantCountServiceTest extends TestCase { @@ -44,31 +43,6 @@ class BookingParticipantCountServiceTest extends TestCase $this->assertSame(7, $bookingDto->participants[7]->index); } - public function testEnsureCorrectNumberOfParticipantsPrepopulatesFreshApplicant(): void - { - $travel = $this->createTravel([1 => 2]); - $bookingDto = new BookingDto($travel, 123); - $bookingDto->roomSelections = [$this->createRoomSelection(1, 1)]; - $bookingDto->participants = [new ParticipantDto()]; - - $user = $this->createMock(UserInterface::class); - $callbackCalled = false; - - $this->service->ensureCorrectNumberOfParticipants( - $bookingDto, - $user, - function (UserInterface $userArg, ParticipantDto $participant) use (&$callbackCalled): ParticipantDto { - $callbackCalled = true; - $participant->firstName = 'Alex'; - - return $participant; - } - ); - - $this->assertTrue($callbackCalled); - $this->assertSame('Alex', $bookingDto->participants[0]->firstName); - } - private function createTravel(array $roomCapacities): Travel { $travel = new Travel(); diff --git a/tests/Service/ParticipantPrepopulationServiceTest.php b/tests/Service/ParticipantPrepopulationServiceTest.php index ea778ce..6757298 100644 --- a/tests/Service/ParticipantPrepopulationServiceTest.php +++ b/tests/Service/ParticipantPrepopulationServiceTest.php @@ -10,9 +10,11 @@ use App\BusProNet\Model\Communication; use App\BusProNet\Model\Notification; use App\BusProNet\Model\PersonalData; use App\Entity\User; +use App\Form\Model\BookingDto; use App\Form\Model\ParticipantDto; use App\Security\Crypt; use App\Service\ParticipantPrepopulationService; +use Carbon\CarbonImmutable; use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; @@ -240,6 +242,44 @@ class ParticipantPrepopulationServiceTest extends TestCase $this->assertTrue($result->bulkInsuranceBooking); } + public function testShouldPrepopulateApplicantOnlyWhenFresh(): void + { + $fresh = new ParticipantDto(); + $fresh->firstName = ''; + + $filled = new ParticipantDto(); + $filled->firstName = 'Already set'; + + $this->assertTrue($this->service->shouldPrepopulateApplicant($fresh)); + $this->assertFalse($this->service->shouldPrepopulateApplicant($filled)); + } + + public function testDummyTokenMatchesOnlyInCreateMode(): void + { + $participant = new ParticipantDto(); + $participant->lastName = ParticipantPrepopulationService::TOKEN; + + $this->assertTrue($this->service->isDummyDataFillRequested($participant, BookingDto::MODE_CREATE)); + $this->assertFalse($this->service->isDummyDataFillRequested($participant, BookingDto::MODE_EDIT)); + } + + public function testFillDummyParticipantSetsGeneratedData(): void + { + CarbonImmutable::setTestNow(CarbonImmutable::create(2026, 1, 15, 14, 30)); + + $participant = new ParticipantDto(); + $this->service->fillDummyParticipant($participant, 0); + + $this->assertSame('Vorname 1', $participant->firstName); + $this->assertSame('Muster 1 14:30', $participant->lastName); + $this->assertSame('0171/111111', $participant->mobile); + $this->assertSame('teilnehmer.in1@example.com', $participant->email); + $this->assertSame('2006-01-15', $participant->dateOfBirth?->format('Y-m-d')); + $this->assertSame('Musterstr. 123', $participant->address?->street); + + CarbonImmutable::setTestNow(); + } + private function createUser(string $email, string $encryptedPassword): User { $user = new User($email);