From d719b17aecdd816a65d6962a1cd5eea9077ca20a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Fromme?= Date: Fri, 25 Jul 2025 09:28:13 +0200 Subject: [PATCH] wip: form refactoring --- .../Booking/CreateStep1Controller.php | 12 ++- .../Booking/CreateStep2Controller.php | 3 +- src/Form/BookingCreateParticipantType.php | 25 ++----- src/Form/BookingEditParticipantType.php | 35 +++++---- src/Form/Model/BookingCreateDto.php | 5 -- src/Form/Model/BookingDtoInterface.php | 18 +---- src/Form/Model/BookingEditDto.php | 16 ++-- .../ParticipantAssignedRoomFieldHandler.php | 1 - .../ParticipantFieldHandlerRegistry.php | 1 - src/Form/Service/EditFieldStateProvider.php | 10 ++- src/Form/Service/FormTraversalTrait.php | 42 +++++++++++ .../ParticipantFieldOptionsProvider.php | 17 +++-- src/Service/BookingService.php | 73 +++++++++++++++---- 13 files changed, 158 insertions(+), 100 deletions(-) create mode 100644 src/Form/Service/FormTraversalTrait.php diff --git a/src/Controller/Booking/CreateStep1Controller.php b/src/Controller/Booking/CreateStep1Controller.php index 8c5f73c..8811a95 100644 --- a/src/Controller/Booking/CreateStep1Controller.php +++ b/src/Controller/Booking/CreateStep1Controller.php @@ -23,8 +23,7 @@ class CreateStep1Controller extends AbstractController public function __construct( private readonly BookingService $bookingService, - ) - { + ) { } /** @@ -34,6 +33,10 @@ class CreateStep1Controller extends AbstractController public function index(Request $request): Response { $bookingCreateDto = $this->bookingService->getOrCreateBookingCreateDto($request); + + // Capture the current room selection state before form processing + $oldRoomSelectionSnapshot = $this->bookingService->createRoomSelectionSnapshot($bookingCreateDto); + $summary = $this->bookingService->getRoomSummaryAndParticipantCount($bookingCreateDto); // Validate step access - allow step 1 or redirect to current step @@ -46,6 +49,9 @@ class CreateStep1Controller extends AbstractController $form->handleRequest($request); if ($form->isSubmitted() && $form->isValid()) { + if ($this->bookingService->hasRoomSelectionChanged($oldRoomSelectionSnapshot, $bookingCreateDto)) { + $this->bookingService->resetParticipantAssignments($bookingCreateDto); + } $bookingCreateDto->currentStep = 2; $this->bookingService->saveBookingCreateDto($request, $bookingCreateDto); @@ -80,8 +86,6 @@ class CreateStep1Controller extends AbstractController ]); $form->handleRequest($request); - $this->bookingService->saveBookingCreateDto($request, $bookingCreateDto); - $summary = $this->bookingService->getRoomSummaryAndParticipantCount($bookingCreateDto); $availableRooms = $bookingCreateDto->travel->getAvailableRooms(); $groupedSelectedRooms = $this->bookingService->groupRoomSelectionsByType($summary['selectedRooms'], $availableRooms); diff --git a/src/Controller/Booking/CreateStep2Controller.php b/src/Controller/Booking/CreateStep2Controller.php index c63cccd..8dcceb6 100644 --- a/src/Controller/Booking/CreateStep2Controller.php +++ b/src/Controller/Booking/CreateStep2Controller.php @@ -27,8 +27,7 @@ class CreateStep2Controller extends AbstractController public function __construct( private readonly BookingService $bookingService, - ) - { + ) { } /** diff --git a/src/Form/BookingCreateParticipantType.php b/src/Form/BookingCreateParticipantType.php index 6213c9b..de4d6b3 100644 --- a/src/Form/BookingCreateParticipantType.php +++ b/src/Form/BookingCreateParticipantType.php @@ -3,9 +3,8 @@ namespace App\Form; use App\BusProNet\Form\CountryType; -use App\Form\Model\BookingCreateDto; -use App\Form\Model\ParticipantDto; use App\Form\Model\BookingDtoInterface; +use App\Form\Model\ParticipantDto; use App\Form\Service\ParticipantFieldOptionsProvider; use Symfony\Component\Form\AbstractType; use Symfony\Component\Form\Extension\Core\Type\BirthdayType; @@ -85,14 +84,12 @@ class BookingCreateParticipantType extends AbstractType return; } - // Traverse up the form tree to get the root form's data. - $rootForm = $form; - while ($rootForm->getParent()) { - $rootForm = $rootForm->getParent(); - } + // Get the booking DTO from the root form + $bookingDto = $this->fieldOptionsProvider->getBookingDtoFromForm($form); - /** @var BookingDtoInterface $bookingDto */ - $bookingDto = $rootForm->getData(); + if (null === $bookingDto) { + return; + } $this->addDynamicFields($form, $bookingDto, $participantData->index); $this->applyFieldStates($form, $bookingDto, $participantData->index); @@ -110,14 +107,8 @@ class BookingCreateParticipantType extends AbstractType return; } - // Get the root form data to access BookingDtoInterface - $rootForm = $form; - while ($rootForm->getParent()) { - $rootForm = $rootForm->getParent(); - } - - /** @var BookingDtoInterface $bookingDto */ - $bookingDto = $rootForm->getData(); + // Get the booking DTO from the root form + $bookingDto = $this->fieldOptionsProvider->getBookingDtoFromForm($form); if (null === $bookingDto) { return; diff --git a/src/Form/BookingEditParticipantType.php b/src/Form/BookingEditParticipantType.php index f6ffa82..cd8f8c9 100644 --- a/src/Form/BookingEditParticipantType.php +++ b/src/Form/BookingEditParticipantType.php @@ -7,7 +7,6 @@ use App\BusProNet\Form\CountryType; use App\BusProNet\Model\Pickup; use App\BusProNet\Model\Service; use App\Form\Model\ParticipantDto; -use App\Form\Model\BookingDtoInterface; use App\Form\Service\EditFieldStateProvider; use Symfony\Component\Form\AbstractType; use Symfony\Component\Form\Extension\Core\Type\BirthdayType; @@ -23,7 +22,8 @@ class BookingEditParticipantType extends AbstractType { public function __construct( private readonly EditFieldStateProvider $fieldStateProvider, - ) {} + ) { + } public function buildForm(FormBuilderInterface $builder, array $options): void { @@ -39,16 +39,15 @@ class BookingEditParticipantType extends AbstractType $form = $event->getForm(); - // Traverse up the form tree to get the root form's data (BookingEditDto) - $rootForm = $form; - while ($rootForm->getParent()) { - $rootForm = $rootForm->getParent(); + // Get the booking DTO from the root form + $bookingDto = $this->fieldStateProvider->getBookingDtoFromForm($form); + + if (null === $bookingDto) { + return; } - /** @var BookingDtoInterface $bookingDto */ - $bookingDto = $rootForm->getData(); // Helper to get state for a field - $getState = fn(string $field) => $this->fieldStateProvider->getFieldState($field, $bookingDto, $participantIndex); + $getState = fn (string $field) => $this->fieldStateProvider->getFieldState($field, $bookingDto, $participantIndex); $form ->add('firstName', TextType::class, $this->mergeFieldState([ @@ -248,7 +247,7 @@ class BookingEditParticipantType extends AbstractType $pickupLabel = $pickup->city; if (null !== $pickup->street) { - $pickupLabel .= ' (' . $pickup->street . ')'; + $pickupLabel .= ' ('.$pickup->street.')'; } $price = $pickup->price; @@ -285,21 +284,20 @@ class BookingEditParticipantType extends AbstractType return; } - // Traverse up the form tree to get the root form's data (BookingEditDto) - $rootForm = $form; - while ($rootForm->getParent()) { - $rootForm = $rootForm->getParent(); + // Get the booking DTO from the root form + $bookingDto = $this->fieldStateProvider->getBookingDtoFromForm($form); + + if (null === $bookingDto) { + return; } - /** @var BookingDtoInterface $bookingDto */ - $bookingDto = $rootForm->getData(); $participantIndex = $form->getData()->index; // Helper to get state for a field based on submitted data - $getState = fn(string $field) => $this->fieldStateProvider->getFieldState($field, $bookingDto, $participantIndex, $data); + $getState = fn (string $field) => $this->fieldStateProvider->getFieldState($field, $bookingDto, $participantIndex, $data); // Re-apply field states to all personal data fields $personalDataFields = [ - 'firstName', 'lastName', 'dateOfBirth', 'gender', 'nationality', 'email', 'mobile' + 'firstName', 'lastName', 'dateOfBirth', 'gender', 'nationality', 'email', 'mobile', ]; foreach ($personalDataFields as $field) { if ($form->has($field)) { @@ -389,6 +387,7 @@ class BookingEditParticipantType extends AbstractType $fieldOptions[$key] = $value; } } + return $fieldOptions; } } diff --git a/src/Form/Model/BookingCreateDto.php b/src/Form/Model/BookingCreateDto.php index 3291694..d2626b2 100644 --- a/src/Form/Model/BookingCreateDto.php +++ b/src/Form/Model/BookingCreateDto.php @@ -40,11 +40,6 @@ class BookingCreateDto implements BookingDtoInterface return $this->participants; } - public function getTravel(): Travel - { - return $this->travel; - } - public function hasParticipant(int $index): bool { return isset($this->participants[$index]); diff --git a/src/Form/Model/BookingDtoInterface.php b/src/Form/Model/BookingDtoInterface.php index 6369a05..4df70e2 100644 --- a/src/Form/Model/BookingDtoInterface.php +++ b/src/Form/Model/BookingDtoInterface.php @@ -4,8 +4,6 @@ declare(strict_types=1); namespace App\Form\Model; -use App\BusProNet\Model\Travel; - /** * Interface for unified access to booking data across create and edit workflows. * @@ -23,13 +21,6 @@ interface BookingDtoInterface */ public function getParticipants(): array; - /** - * Gets the travel data for the booking. - * - * @return Travel The travel data containing services, hotels, and other booking options - */ - public function getTravel(): Travel; - /** * Checks if a participant exists at the given index. * @@ -47,11 +38,4 @@ interface BookingDtoInterface * @return ParticipantDto|null The participant DTO or null if not found */ public function getParticipant(int $index): ?ParticipantDto; - - /** - * Gets all selected rooms for the booking. - * - * @return array Array of selected room DTOs (may be empty for edit DTOs) - */ - public function getSelectedRooms(): array; -} \ No newline at end of file +} diff --git a/src/Form/Model/BookingEditDto.php b/src/Form/Model/BookingEditDto.php index 443b2ba..688b992 100644 --- a/src/Form/Model/BookingEditDto.php +++ b/src/Form/Model/BookingEditDto.php @@ -10,18 +10,19 @@ use Symfony\Component\Validator\Constraints as Assert; class BookingEditDto implements BookingDtoInterface { - public ?Booking $booking = null; - public ?Travel $travel = null; - /** * @var array */ #[Assert\Valid] public array $participants = []; + public function __construct(public Booking $booking, public Travel $travel) + { + } + public static function fromBooking(Booking $booking, Travel $travel): static { - $instance = new static(); + $instance = new static($booking, $travel); $instance->booking = $booking; $instance->travel = $travel; @@ -69,11 +70,6 @@ class BookingEditDto implements BookingDtoInterface return $this->participants; } - public function getTravel(): Travel - { - return $this->travel; - } - public function hasParticipant(int $index): bool { return isset($this->participants[$index]); @@ -87,7 +83,7 @@ class BookingEditDto implements BookingDtoInterface /** * Gets all selected rooms for the booking (edit context). * - * @return array Always returns an empty array for edit DTOs unless implemented. + * @return array always returns an empty array for edit DTOs unless implemented */ public function getSelectedRooms(): array { diff --git a/src/Form/ParticipantFieldHandler/ParticipantAssignedRoomFieldHandler.php b/src/Form/ParticipantFieldHandler/ParticipantAssignedRoomFieldHandler.php index d0566c2..2f97aec 100644 --- a/src/Form/ParticipantFieldHandler/ParticipantAssignedRoomFieldHandler.php +++ b/src/Form/ParticipantFieldHandler/ParticipantAssignedRoomFieldHandler.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace App\Form\ParticipantFieldHandler; -use App\Form\Model\BookingCreateDto; use App\Form\Model\BookingDtoInterface; /** diff --git a/src/Form/ParticipantFieldHandler/ParticipantFieldHandlerRegistry.php b/src/Form/ParticipantFieldHandler/ParticipantFieldHandlerRegistry.php index 9ffa615..5eea8ed 100644 --- a/src/Form/ParticipantFieldHandler/ParticipantFieldHandlerRegistry.php +++ b/src/Form/ParticipantFieldHandler/ParticipantFieldHandlerRegistry.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace App\Form\ParticipantFieldHandler; -use App\Form\Model\BookingCreateDto; use App\Form\Model\BookingDtoInterface; /** diff --git a/src/Form/Service/EditFieldStateProvider.php b/src/Form/Service/EditFieldStateProvider.php index 9d73d22..f8986d7 100644 --- a/src/Form/Service/EditFieldStateProvider.php +++ b/src/Form/Service/EditFieldStateProvider.php @@ -5,10 +5,10 @@ declare(strict_types=1); namespace App\Form\Service; use App\Form\Model\BookingDtoInterface; -use App\Form\ParticipantFieldHandler\Condition\FieldConditionInterface; -use App\Form\ParticipantFieldHandler\Condition\MutabilityCondition; use App\Form\ParticipantFieldHandler\Condition\ApplicantCondition; use App\Form\ParticipantFieldHandler\Condition\CompositeCondition; +use App\Form\ParticipantFieldHandler\Condition\FieldConditionInterface; +use App\Form\ParticipantFieldHandler\Condition\MutabilityCondition; /** * Field state provider for the booking edit workflow. @@ -21,6 +21,8 @@ use App\Form\ParticipantFieldHandler\Condition\CompositeCondition; */ class EditFieldStateProvider implements FieldStateProviderInterface { + use FormTraversalTrait; + /** @var array> */ private array $fieldStateConditions = []; @@ -109,6 +111,7 @@ class EditFieldStateProvider implements FieldStateProviderInterface foreach ($this->fieldStateConditions[$fieldName] as $condition) { $dependencies = array_merge($dependencies, $condition->getDependentFields()); } + return array_unique($dependencies); } @@ -121,6 +124,7 @@ class EditFieldStateProvider implements FieldStateProviderInterface $allStates[$fieldName] = $fieldState; } } + return $allStates; } -} \ No newline at end of file +} diff --git a/src/Form/Service/FormTraversalTrait.php b/src/Form/Service/FormTraversalTrait.php new file mode 100644 index 0000000..ae45966 --- /dev/null +++ b/src/Form/Service/FormTraversalTrait.php @@ -0,0 +1,42 @@ +getParent()) { + $rootForm = $rootForm->getParent(); + } + + $data = $rootForm->getData(); + + return $data instanceof BookingDtoInterface ? $data : null; + } +} diff --git a/src/Form/Service/ParticipantFieldOptionsProvider.php b/src/Form/Service/ParticipantFieldOptionsProvider.php index 17f6553..ca2594b 100644 --- a/src/Form/Service/ParticipantFieldOptionsProvider.php +++ b/src/Form/Service/ParticipantFieldOptionsProvider.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace App\Form\Service; +use App\Form\Model\BookingCreateDto; use App\Form\Model\BookingDtoInterface; use App\Form\ParticipantFieldHandler\Condition\FieldConditionInterface; @@ -26,6 +27,8 @@ use App\Form\ParticipantFieldHandler\Condition\FieldConditionInterface; */ class ParticipantFieldOptionsProvider implements FieldStateProviderInterface { + use FormTraversalTrait; + /** @var array Field option providers indexed by field name */ private array $fieldOptionProviders = []; @@ -120,7 +123,7 @@ class ParticipantFieldOptionsProvider implements FieldStateProviderInterface */ private function registerFieldOptionProviders(): void { - // Room assignment field provider + // Room assignment field provider (only available for create workflow) $this->fieldOptionProviders['assignedRoomId'] = fn (BookingDtoInterface $bookingDto, int $participantIndex) => [ 'label' => 'Zimmer', 'placeholder' => 'Bitte wählen', @@ -128,11 +131,13 @@ class ParticipantFieldOptionsProvider implements FieldStateProviderInterface // - Shows only available rooms for this participant // - Excludes rooms already assigned to other participants // - Respects room capacity and booking constraints - 'choice_loader' => $this->roomChoiceLoaderFactory->create( - $bookingDto->participants, - $bookingDto->getSelectedRooms(), - $participantIndex - ), + 'choice_loader' => $bookingDto instanceof BookingCreateDto + ? $this->roomChoiceLoaderFactory->create( + $bookingDto->participants, + $bookingDto->getSelectedRooms(), + $participantIndex + ) + : null, ]; // Future field providers would be added here, for example: diff --git a/src/Service/BookingService.php b/src/Service/BookingService.php index 847aab9..990763b 100644 --- a/src/Service/BookingService.php +++ b/src/Service/BookingService.php @@ -6,7 +6,6 @@ use App\BusProNet\Model\Room; use App\BusProNet\Model\Travel; use App\Form\Model\BookingCreateDto; use App\Form\Model\RoomSelectionDto; -use App\Service\TravelDataService; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; @@ -14,8 +13,7 @@ class BookingService { public function __construct( private readonly TravelDataService $travelDataService, - ) { - } + ) {} public function getOrCreateBookingCreateDto(Request $request): BookingCreateDto { @@ -43,7 +41,7 @@ class BookingService $availableRooms = $travelData->getAvailableRooms(); $roomSelections = array_map( - fn (Room $room) => $this->createRoomSelection($room, $roomsIdsAndQuantities), + fn(Room $room) => $this->createRoomSelection($room, $roomsIdsAndQuantities), $availableRooms ); @@ -103,7 +101,6 @@ class BookingService return $participantsCount; } - /** * Calculates the number of participants assigned to each room ID. * @@ -124,7 +121,6 @@ class BookingService /** * Returns a summary of selected rooms and the resulting participant count for a booking. * - * @param BookingCreateDto $bookingCreateDto * @return array{selectedRooms: array, participantCount: int} */ public function getRoomSummaryAndParticipantCount(BookingCreateDto $bookingCreateDto): array @@ -142,19 +138,20 @@ class BookingService * Groups available rooms by selection type ('by_pax' or 'by_room'). * * @param array $rooms Rooms indexed by room ID + * * @return array{by_pax: array, by_room: array} */ public function groupRoomsBySelectionType(array $rooms): array { $groups = [ - 'by_pax' => [], - 'by_room' => [], + Room::SELECTION_TYPE_BY_PAX => [], + Room::SELECTION_TYPE_BY_ROOM => [], ]; foreach ($rooms as $room) { - if (stripos($room->label, 'bett') !== false) { - $groups['by_pax'][$room->id] = $room; + if (false !== stripos($room->label, 'bett')) { + $groups[Room::SELECTION_TYPE_BY_PAX][$room->id] = $room; } else { - $groups['by_room'][$room->id] = $room; + $groups[Room::SELECTION_TYPE_BY_ROOM][$room->id] = $room; } } @@ -164,15 +161,16 @@ class BookingService /** * Groups roomSelections by selection type ('by_pax' or 'by_room'), using Room::getSelectionType(). * - * @param array $roomSelections Array of selected RoomSelectionDto - * @param array $roomsById Rooms indexed by room ID + * @param array $roomSelections Array of selected RoomSelectionDto + * @param array $roomsById Rooms indexed by room ID + * * @return array{by_pax: array, by_room: array} */ public function groupRoomSelectionsByType(array $roomSelections, array $roomsById): array { $groups = [ - 'by_pax' => [], - 'by_room' => [], + Room::SELECTION_TYPE_BY_PAX => [], + Room::SELECTION_TYPE_BY_ROOM => [], ]; foreach ($roomSelections as $roomSelection) { $room = $roomsById[$roomSelection->roomId] ?? null; @@ -181,7 +179,50 @@ class BookingService $groups[$type][] = $roomSelection; } } - + return $groups; } + + /** + * Determines if the room selection has changed between two DTOs. + */ + public function shouldResetAssignments(BookingCreateDto $oldDto, BookingCreateDto $newDto): bool + { + $old = array_map(fn($roomSelectionDto) => [$roomSelectionDto->roomId, $roomSelectionDto->quantity], $oldDto->roomSelections); + $new = array_map(fn($roomSelectionDto) => [$roomSelectionDto->roomId, $roomSelectionDto->quantity], $newDto->roomSelections); + + return $old !== $new; + } + + /** + * Resets all participant room assignments in the DTO. + */ + public function resetParticipantAssignments(BookingCreateDto $dto): void + { + foreach ($dto->participants as $participant) { + $participant->assignedRoomId = null; + } + } + + /** + * Creates a snapshot of the current room selection state. + * + * @return array Array of [roomId, quantity] pairs + */ + public function createRoomSelectionSnapshot(BookingCreateDto $dto): array + { + return array_map( + fn($roomSelection) => [$roomSelection->roomId, $roomSelection->quantity], + $dto->roomSelections + ); + } + + /** + * Checks if room selection has changed compared to a previous snapshot. + */ + public function hasRoomSelectionChanged(array $oldSnapshot, BookingCreateDto $newDto): bool + { + $newSnapshot = $this->createRoomSelectionSnapshot($newDto); + return $oldSnapshot !== $newSnapshot; + } }